civiform / civiform

CiviForm simplifies the application process for government benefits programs by re-using applicant data for multiple benefits applications. It's being developed by Google.org and Exygy, in collaboration with the City of Seattle and community contributors.
https://civiform.us
Apache License 2.0
90 stars 56 forks source link

Client-Side File Upload Validation Bypass #6982

Open dkatzz opened 4 months ago

dkatzz commented 4 months ago

Describe the bug The program's image upload functionality relies on client-side file type checks, which can be bypassed. This allows an attacker to upload potentially harmful files disguised with permitted extensions.

Recommendations

dkatzz commented 4 months ago

@caitlinshk I remember chatting about something similar and it being difficult to validate anything on the server side because we send the file straight to AWS and their error messaging isn't great, right?

caitlinshk commented 4 months ago

It is actually possible to have AWS validate the file type, we just need to set it up - see https://github.com/civiform/civiform/issues/6301.

It's true that if a user bypasses the client-side type check and uploads a disallowed filetype, then they'll see an ugly AWS error and not be redirected back to CiviForm. But, that seems fine since the people doing that would likely be malicious attackers.

gwendolyngoetz commented 4 months ago

Also related to https://github.com/civiform/civiform/issues/5218

nb1701 commented 2 months ago

There's another small bug here, in that even when client side validation catches that it's invalid, the save image button is enabled and then ends up sending nothing back, resulting in an error from AWS because the file has size zero.

nb1701 commented 2 months ago

Additionally, you can't test the fix here locally, because LocalStack does not appear to respect the Content-Type condition.

dkatzz commented 2 months ago

Additionally, you can't test the fix here locally, because LocalStack does not appear to respect the Content-Type condition.

It may be worth filing an issue against localstack. They've fixed issues we've seen in the past (https://github.com/localstack/localstack/issues/10351)

nb1701 commented 2 months ago

This isn't going to be quite as easy as I'd hoped. While we can add a Content-Type restriction to the policy in the presigned upload URL, since we're submitting it through a form, the type on the POST request is multipart/form-data, and apparently AWS is only looking at the header to determine if the data is allowed.

We may need to do something like let it upload, and then on the callback, check the MIME type on the file just uploaded, and delete it if it is invalid. Which is pretty cumbersome.

nb1701 commented 2 months ago

Some more notes:

Still not sure what the best way forward is here.

nb1701 commented 1 month ago

Going to investigate post-upload validation. We maybe be able to download perhaps just the first 1kb of the file and pass it through https://tika.apache.org/ and delete the file if it is not an expected type.

nb1701 commented 1 month ago

After some more thought, possibly all we need to do is ensure the file has a valid extension. For MacOS/Linux, a file can't be executed unless you chmod +x it, so if a malicious executable is uploaded, double clicking it isn't going to do anything. Windows relies on the file extension to determine how to open it, so as long as we ensure all files have a valid extension type, we should be okay there.