Skip to content

Bug in OutputWindow.cs that will cause ZipInputStream.Read() to never run out of data in certain corrupt ZIP files #19

Description

@klavatar

HI there, I recently discovered a potential bug in Zip/Compression/Streams/OutputWindow.cs that, when combined with a corrupt ZIP archive, may cause ZipInputStream.Read() to never run out of data.

Specifically, in line 208 (version 0.86.0.518):

    copyEnd = (windowEnd - windowFilled + len) & WindowMask;

Where copyEnd is supposed to be a positive index into the window. However, given a ZIP archive that is corrupt in a particular way (I have an example ZIP file that triggers this behavior), (windowEnd - windowFilled + len) will become negative. However, the "& WindowMask" operator masks off the high bits, which converts the number back to positive. This throws off the subsequent logic and will set the code into an infinite loop where the ZipInputStream.Read() calls always returns "new" data, and therefore never return "0".

So I made a simple modification that first checks to see if (windowEnd - windowFilled + len) is negative. If that's the case, I simply throw a "WIndow overflow" error, something like this.

            int endOffset = windowEnd - windowFilled + len;
            if (endOffset < 0) {
                throw new InvalidOperationException("Window overflow");
            }
            copyEnd = endOffset & WindowMask;

This seems to correctly detect the corruption and throw an exception.

I think there must be more elegant ways to fix this, but this quick-and-dirty trick did it for me. I wnt to re-iterate that this only happens (as far as I know) when the ZIP file is corrupt, but since I have to deal with ZIP files that are created by others, this does come in handy. Hope this might be helpful to someone.

  • K.

Activity

  1. self-assigned this
    on Apr 14, 2016
  2. ayushman4 commented on Jun 21, 2018

    @ayushman4

    @McNeight Is this issue on the roadmap to be fixed?

  3. piksel commented on Jun 21, 2018

    @piksel
    Member

    @ayushman4 Do you currently have a sample case for this?
    If so you could try #233 that should prevent this by not allowing bad headers (although this bug should probably be addressed as well).

  4. self-assigned this
    on Jul 1, 2018
  5. added
    zipRelated to ZIP file format
    on Jul 1, 2018
  6. piksel commented on Jul 24, 2018

    @piksel
    Member

    @akshaysonatype I wonder this too. I have been unable to reproduce this, nor figure out what kind of bad data would cause this to happen. The described behaviour seems unlikely giving the current codebase, but this was reported for 0.86, so there might have been some changes since that.

  7. piksel commented on Sep 16, 2018

    @piksel
    Member

    Closing this due to inactivity and age.

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

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions