uncompressed size is not enforced by zlib.createInflateRaw() or afterwards by yauzl

[jorangreef]

@thejoshwolfe thanks for a brilliant module.

Firstly, and this is more something that's lacking in the zlib api which we should change, but I noticed that yauzl doesn't let createInflateRaw know that it only expects the uncompressed buffer to be at most uncompressed size bytes, and that zlib should stop decoding and throw an error once it hits the uncompressed size limit.

Secondly, yauzl doesn't check the inflated buffer length returned by zlib to ensure that it matches the uncompressed size exactly.

Simply checking uncompressed size alone before inflating is not enough, since uncompressed size is untrusted user data and could be fraudulent, i.e. much less than that indicated by the actual zlib stream.

Both of these protections are necessary for defending against zip bombs, otherwise the user might check that uncompressed size is within limits, yet zlib might carry on decoding hundreds of megabytes past this limit. And if yauzl sees that the sizes do not match up after the fact, then it should probably warn the user, otherwise multiple instances of this mismatch might add up to something more severe across multiple files.

[thejoshwolfe]

Did you see the documentation in the readme about validateEntrySizes? I'm pretty sure I'm doing exactly what you want already. It's even tested by this test case: https://github.com/thejoshwolfe/yauzl/blob/master/test/failure/too%20many%20bytes%20in%20the%20stream%20expected%2082496%20got%20at%20least%2098304.zip

This was implemented in #13 and released in version 2.3.0 in 2015.

[jorangreef]

Thanks @thejoshwolfe, I confess I didn't read the README, but I did read through index.js several times. I was focusing on createInflateRaw() and I missed the AssertByteCountStream being piped after the fact.

Another idea is for yauzl to count the bytes flowing out of zlib and make sure it matches the uncompressedSize reported in the metadata. If we get too many bytes (or too few bytes), end the streams and emit an error. With this approach, you could check for probable zip bombs by simply checking the uncompressedSize field and listening for errors. Would this be acceptable for your requirements?

Yes, this is exactly what I had in mind from the yauzl side.

All good then, I think it would still be good for Node's zlib to accept a maxUncompressedSize option.