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

V12.1.2 check maximum uncomressed file size #799

Closed
elarlang opened this issue May 30, 2020 · 18 comments
Closed

V12.1.2 check maximum uncomressed file size #799

elarlang opened this issue May 30, 2020 · 18 comments
Assignees
Milestone

Comments

@elarlang
Copy link
Collaborator

V12.1.2

Current requirement:

V12.1.2 Verify that compressed files are checked for "zip bombs" - small input files that will decompress into huge files thus exhausting file storage limits.

Problems:

  • "zip bombs" is just a name, but still leads to zip format. Problem is general for "compressed files", including docx, xlsx etc.
  • "... thus exhausting file storage limits" - this is only valid, when server unpack compressed files on the server side. Uploading "zip bombs" for other users to download gives also attack vector against site visitors/users.

Proposal ideas (but not wording):

  • "Verify that the application checks compressed files against maximum allowed uncompressed size"
  • We still should mention "ZIP bomb" as an example attack

Question:

  • Should we address also "maximum allowed amount of files in compressed container" limit?
@jmanico
Copy link
Member

jmanico commented May 31, 2020 via email

@tghosth
Copy link
Collaborator

tghosth commented Jun 2, 2020

@elarlang agree with "Proposal", please provide proposed wording.
Not sure I understand the "Question". What is the risk?

@tghosth tghosth added the 2) Awaiting response Awaiting a response from the original poster label Jun 2, 2020
@elarlang
Copy link
Collaborator Author

elarlang commented Jun 2, 2020

Situation - uploaded ZIP (or whatever container) file is uploaded, file contains "max amount of allowed files in ZIP" container files and those will be unpacked to one folder. And then few more of this kind of files. From some moment I may take too much resources to have file listing from that folder and it may cause dos attack because of that.

@tghosth
Copy link
Collaborator

tghosth commented Jun 2, 2020

Ok, happy to see a proposed wording. Separate requirement?

@elarlang
Copy link
Collaborator Author

elarlang commented Jun 10, 2020

Proposals for discussion:

  • Plain

Verify that the application checks compressed files against maximum allowed uncompressed size and against maximum files inside container.

  • Extensions

Verify that the application checks compressed files (e.g. zip, gz, docx, odt) against maximum allowed uncompressed size and against maximum files inside container.

  • Explanations

Verify that the application checks compressed files (e.g. zip, gz, docx, odt) against maximum allowed uncompressed size and against maximum files inside container to avoid "ZIP bomb" attacks against the application and users.

@jmanico
Copy link
Member

jmanico commented Jun 10, 2020 via email

@elarlang
Copy link
Collaborator Author

elarlang commented Jun 10, 2020

With running unzip you already got the attack (even if it does not fill your disk quota, it's using resources for unpacking).

What about:

$ zipinfo test.zip 
Archive:  test.zip
Zip file size: 101940 bytes, number of entries: 1
-rw-r--r--  3.0 unx 104857600 tx defX 20-Mar-20 08:59 test.pdf
1 file, 104857600 bytes uncompressed, 101774 bytes compressed:  99.9%

and

$ exiftool test.zip 
ExifTool Version Number         : 10.80
File Name                       : test.zip
Directory                       : .
File Size                       : 100 kB
File Modification Date/Time     : 2020:03:20 08:59:58+02:00
File Access Date/Time           : 2020:06:10 15:48:45+03:00
File Inode Change Date/Time     : 2020:03:23 10:41:51+02:00
File Permissions                : rw-r--r--
File Type                       : ZIP
File Type Extension             : zip
MIME Type                       : application/zip
Zip Required Version            : 20
Zip Bit Flag                    : 0x0002
Zip Compression                 : Deflated
Zip Modify Date                 : 2020:03:20 08:59:29
Zip CRC                         : 0xba4c670c
Zip Compressed Size             : 101774
Zip Uncompressed Size           : 104857600
Zip File Name                   : test.pdf

@jmanico
Copy link
Member

jmanico commented Jun 10, 2020 via email

@elarlang
Copy link
Collaborator Author

elarlang commented Jun 10, 2020

In practice, adding a lot (empty) files increases output zip file size quite fast:

  • 100 000 files = 8.4MB, generated in 6sec
  • 500 000 files = 43.7MB, generated in 30sec

It's possible to decrease file sizes a bit but it gives some idea.

Example (random) solution for detecting "amount of files", yes, it goes through listing, but it shows that time is not something for what you need to take calendar:

time zipinfo -1 alotoffiles_500000.zip | wc -l
500000

real	0m1,724s
user	0m1,171s
sys	0m1,951s

Practical advice could be: allow smaller "max upload file size" for archives from untrusted sources.

So I would say that within relatively small "max upload file size" you will not have "huge catastrophic-problematic amount of files in container" from detection perspective. If you keep unpacking them constantly to the same folder then you will have one.

This is my knowledge at the moment with quick research.

@jmanico
Copy link
Member

jmanico commented Jun 11, 2020 via email

@elarlang
Copy link
Collaborator Author

Can you try 500 000 000 files if you have the machine to handle it?

at the moment it could take too much time (to provide quick answers). But definitely I'll investigate it further.

Also, we still need to be weary of Zip Bombs; 42k zip files that unzip into Petabytes https://en.wikipedia.org/wiki/Zip_bomb

this one requires recursive unpacking. Another tricky topic, should we warn about nested zip and avoid recursive unpacking or don't allow zip inside zip?

@jmanico
Copy link
Member

jmanico commented Jun 11, 2020 via email

@elarlang
Copy link
Collaborator Author

... and based on all that, any proposals to my proposals? :)

@tghosth tghosth removed the 2) Awaiting response Awaiting a response from the original poster label Jun 12, 2020
@vanderaj vanderaj added this to the 4.0.2 milestone Jun 23, 2020
@vanderaj
Copy link
Member

@jmanico I'll take this through to completion if you don't mind. I'm working on backporting these changes to 4.0.2

@vanderaj vanderaj self-assigned this Jun 23, 2020
@jmanico
Copy link
Member

jmanico commented Jul 13, 2020

Ok all yours Andrew

@jmanico jmanico closed this as completed Jul 13, 2020
@jmanico jmanico reopened this Jul 13, 2020
@jmanico jmanico removed their assignment Jul 13, 2020
@tghosth
Copy link
Collaborator

tghosth commented Aug 23, 2020

I think this is going to end up too complicated for 4.0.2

@tghosth tghosth modified the milestones: 4.0.2, 4.1 Aug 23, 2020
@jmanico
Copy link
Member

jmanico commented Aug 23, 2020 via email

@jmanico
Copy link
Member

jmanico commented Mar 11, 2021

My proposal:

12.1.2: Verify that the application checks compressed files (e.g. zip, gz, docx, odt) against maximum allowed uncompressed size and against maximum number of files before uncompressing the file.

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

No branches or pull requests

4 participants