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

deflate.zig: check for distances past beginning of output stream #9860

Merged
merged 1 commit into from
Oct 4, 2021

Conversation

mattbork
Copy link
Contributor

InflateStream doesn't check if the distance in a backward reference extends past the beginning of the decompressed "output stream". For example, if the first block starts by encoding the literals 'A', 'B', and 'C' followed a (length, distance) pair of (4, 4), we will reach one byte past the beginning of the output stream. This will output "ABCxABC" where x is a garbage byte (in this case, whatever happened to be in the last slot of the window buffer). This should not be allowed according to the RFC, and the reference implementation in Mark Adler's puff.c checks for this as well. The fix is simply to have window keep track of the total number of bytes written out, independent of the number of unread elements in el.

@squeek502
Copy link
Collaborator

squeek502 commented Sep 30, 2021

Nice work, this was something I was looking into as well but didn't understand well enough to properly fix. I was trying to use distance > self.window.readable() as the conditional but that was giving true incorrectly once the window was fully read (?; not sure if this is the correct terminology, still don't feel like I fully understand the window).

@mattbork
Copy link
Contributor Author

The window is a simple circular buffer holding the last N bytes that have been decompressed. It's how copies from backward references are implemented (thus if you want to support the maximum backward distance of 32768, make sure the window buffer is at least 32K). Since it's circular, after the first N bytes have been decompressed, byte N+1 will overwrite byte 0, N+2 will over write byte 1, etc. Obviously, we don't want to do this until those old bytes have actually been read out by the user of InflateStream, so the number of unread bytes in the window is tracked in window.el / window.readable(). As these unread bytes are copied out to the buffer passed to InflateStream.read, window.el is decreased and thus window.buf.len - window.el (which is what window.writable() returns), is increased, indicating that there is more free space to continue decompressing. So window.el can't be used for checking that distances are valid. For example, if we fill a 32K window with 32K decompressed bytes but the buffer passed to read is also 32K, then window.el, window.ri, and window.wi all just end up back at 0, and there's nothing to distinguish this state from the initial state before we decompressed anything. Hence we have to keep track of the total number of bytes decompressed to properly check distances.

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.

None yet

3 participants