Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent Upload of ZIP bombs #7407

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tanmaypardeshi
Copy link

Fixes #7406

Short description of what this resolves:

  1. As we can see in the test case, we are fixing CWE 409 in the OpenEMR platform which is “Improper Handling of Highly Compressed Data(Data Amplification)”. This is ASVS 12.1.2 in the ASVS document which says that “Verify that compressed files are checked for "zip bombs" - small input files that will decompress into huge files thus exhausting file storage limits.”
  2. However, when we try to upload a zip bomb on the OpenEMR platform, it allows it under the Admin->Documents->Document Templates section.
  3. The fix was to add code which checks zip bombs on the platform for this particular section of the portal mentioned in the previous point. As you can see in the code diff above, there has been additional code written for .zip extensions and the code performs 2 checks.
  4. The first check is that the zip file cannot be larger than 1MB by setting a variable $maxZipSize = 1048576. This will make sure that the code does not allow large zip files. The $maxZipSize can be set to whatever value seems appropriate for the platform and I have taken 1MB just as an example.
  5. By default OpenEMR has a 64MB limit on the file size which is being uploaded in its settings already. I have added this new check of 1MB just for the zip files. For the other acceptable file formats, the default file size capacity is still 64MB.
  6. The second check is to see if the zip file which is uploaded has more zip files nested under it. If it is so, the code will reject this kind of a file because such files are usually zip bombs.

Changes Proposed in the PR

a. ZIP bombs:

  1. Checking for zip bombs when the file is being uploaded.
  2. If the file type is zip, then decompress it and see if it is a nested zip file.
  3. If the nested zip file is found, do not let the user upload it.
  4. Following is the error thrown when a user tries to upload a nested zip file.
    zip_bomb

b. Large ZIP files

  1. Checking for large zip files when the file is being uploaded.
  2. If the file type is zip, then check for the file size
  3. If the zip file large in size, do not let the user upload it.
  4. Following is the error thrown when a user tries to upload a large zip file.
    large_file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ZIP Bombs and large ZIP files being allowed in file uploads
1 participant