Fix for CWE-434: Unrestricted Upload of File with Dangerous Type

[leika]

🐕 Corgea issued a PR to fix a vulnerability found in src-ui/src/app/components/admin/config/config.component.ts.

It is CWE-434: Unrestricted Upload of File with Dangerous Type that has a severity of 🔴 High.

🪄 Fix explanation

The fix restricts file uploads to specific types (PNG, JPEG, PDF) and limits file size to 5 MB, preventing the upload of potentially dangerous files.
- Introduced "allowedFileTypes" array to specify permissible file types: PNG, JPEG, PDF.
- Added a "maxFileSize" constant to limit file uploads to 5 MB.
- Implemented a check using "allowedFileTypes.includes(file.type)" to validate file type.
- Added a size check "file.size > maxFileSize" to ensure file size does not exceed 5 MB.
- Utilized "this.toastService.showError" to notify users of invalid file types or sizes.

💡 Important Instructions

Ensure that the file variable is correctly defined and accessible in the context where these checks are implemented.

See the issue and fix in Corgea.

[corgea]

🐕 Corgea found a CWE-434: Unrestricted Upload of File with Dangerous Type vulnerability that is rated as 🔴 High.
View in Corgea.

🎟️Issue Explanation: The code checks file types but doesn't prevent dangerous files from being uploaded, risking execution of harmful code.

More issue details

- The check "!allowedFileTypes.includes(file.type)" only filters known types, missing disguised dangerous files.
- An attacker could upload a script with a safe extension, bypassing the check and executing malicious code.
- If the system processes uploaded files, an attacker could exploit this to run harmful operations.

We could not generate a fix for this.

[graphite-app]

The code is placed at the top of the file outside any function or class scope, which will cause syntax errors. These security checks should be moved into the appropriate file upload handler method within the component class.

Consider refactoring as follows:

// Inside the component class
private readonly allowedFileTypes = ['image/png', 'image/jpeg', 'application/pdf'];
private readonly maxFileSize = 5 * 1024 * 1024; // 5 MB

// Inside your file upload handler method
private handleFileUpload(file: File): boolean {
  if (!this.allowedFileTypes.includes(file.type)) {
    this.toastService.showError($localize`Invalid file type`);
    return false;
  }
  if (file.size > this.maxFileSize) {
    this.toastService.showError($localize`File size exceeds the limit of 5 MB`);
    return false;
  }
  return true;
}

This maintains the security improvements while ensuring proper TypeScript structure.

Suggested change

	const allowedFileTypes = ['image/png', 'image/jpeg', 'application/pdf'];
	const maxFileSize = 5 * 1024 * 1024; // 5 MB
	if (!allowedFileTypes.includes(file.type)) {
	this.toastService.showError($localize`Invalid file type`);
	return;
	}
	if (file.size > maxFileSize) {
	this.toastService.showError($localize`File size exceeds the limit of 5 MB`);
	return;
	}
	// Inside the component class
	private readonly allowedFileTypes = ['image/png', 'image/jpeg', 'application/pdf'];
	private readonly maxFileSize = 5 * 1024 * 1024; // 5 MB
	private validateFile(file: File): boolean {
	if (!this.allowedFileTypes.includes(file.type)) {
	this.toastService.showError($localize`Invalid file type`);
	return false;
	}
	if (file.size > this.maxFileSize) {
	this.toastService.showError($localize`File size exceeds the limit of 5 MB`);
	return false;
	}
	return true;
	}

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.