[Bug 58499] - ZipSecureFile throws zip bomb detected

[Axel Howind <ax...@dua3.com>]

> The main purpose of the zip bomb detection is to safely handle untrusted input.
> Since a workbook that has already been fully read into memory has passed the
> safety test, I suppose you could consider it quasi-safe with regard to writing
> it back out.
>
> If you can't think of a way that a latent zip bomb could unwrap itself while
> writing the workbook that wasn't a result of poor (trusted) code, then I would
> agree with removing this limit when writing.
>
> Not being a security expert, the safer option is to set different read and
> write limits.
>
> Let's continue the discussion on the POI dev mailingdev@poi.apache.org <ma...@poi.apache.org>.


As I wrote in a comment to that bug, I had the same issue today. I looked at
the code, and what happens is that data is written to a temporary file. When
SXSSFWorkbook.write() is called, the header is written, and then the data
from the temporary file is read back in and appended.

This is done in the private method SXSSFWorkbook.injectData().

If we can trust that the temporary data has not been tampered with (and in
which case there'd be a much worse problem), I think we can drop the
heuristic for zip bomb detection when reading the data back in.

If we just replace the first line in injectData():

     ZipFile zip = ZipHelper.openZipFile(zipfile);

by

     ZipFile zip = new ZipFile(zipfile);

that should do it.

(ZipHelper also checks that zipfile exists, but if it doesn't that leads to
NPE afterwards eich IMHO is not better than IOException in that case.)

I could create a patch for this, but I'd like to discuss this first.

So what are the other devs opinions about this?

PS: Since this bug is marked RESOLVED FIXED, how should I submit a
patch? Reopen the bug and attach the patch or just mail the patch
to this list?

Re: [Bug 58499] - ZipSecureFile throws zip bomb detected

[Axel Howind <ax...@dua3.com>]

Created Bug 59743 and attached patch.

> > Once there is consensus on how POI should handle this, open a new bug that
> > "depends on" bug 58499.





---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org

Re: [Bug 58499] - ZipSecureFile throws zip bomb detected

[Dominik Stadler <do...@gmx.at>]

I'm fine with using ZipFile directly internally in SWSSFWorkbook. A quick
look did not reveal any glaring security problem with doing this. We mostly
prevent denial-of-service types of attacks here so if a malicious user can
overwrite the temporary file he can do lots of other bad things anyway...

Dominik.

On Tue, Jun 21, 2016 at 12:18 AM, Javen O'Neal <on...@apache.org> wrote:

> On Jun 20, 2016 2:41 PM, "Axel Howind" <ax...@dua3.com> wrote:
> > How should I submit a patch?
>
> Once there is consensus on how POI should handle this, open a new bug that
> "depends on" bug 58499.
>

Re: [Bug 58499] - ZipSecureFile throws zip bomb detected

[Javen O'Neal <on...@apache.org>]

On Jun 20, 2016 2:41 PM, "Axel Howind" <ax...@dua3.com> wrote:
> How should I submit a patch?

Once there is consensus on how POI should handle this, open a new bug that
"depends on" bug 58499.