Skip to content

Fix calculation for using the "fast path" with Deflate#25

Merged
dunhor merged 12 commits into
mainfrom
dunhor/oob_read
Jun 26, 2026
Merged

Fix calculation for using the "fast path" with Deflate#25
dunhor merged 12 commits into
mainfrom
dunhor/oob_read

Conversation

@dunhor

@dunhor dunhor commented Jun 25, 2026

Copy link
Copy Markdown
Member

A prior optimization I made changed the bitstream code to only do a single < 16 check for the unchecked reading of data and then always read two bytes. This means that the previous size check for continuing down the fast path was no longer accurate and needs to be updated, in this case both Deflate and Deflate64 need 8 bytes.

New tests added that stress these scenarios and I made an attempt to augment the fuzz tests to test these scenarios, however I have yet to hit an actual failure yet, even without this change.

@dunhor dunhor requested a review from DHowett June 25, 2026 21:50
Comment thread src/lib/huffman_tree.c Fixed
Comment thread test/cpp/InflateTests.cpp Outdated
Comment thread src/include/inflatelib.h Outdated

@DHowett DHowett left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only nits - happy to double tap if you fix 'em

Comment thread src/include/inflatelib.h Outdated
struct inflatelib_state; /* Opaque to client applications */

/*
* This struct stores all data and state for decompressing Defalte/Deflate64 encoded data. With the exception of

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* This struct stores all data and state for decompressing Defalte/Deflate64 encoded data. With the exception of
* This struct stores all data and state for decompressing Deflate/Deflate64 encoded data. With the exception of

nit

Comment thread src/include/inflatelib.h Outdated
/*
*
* "Inflates" the Deflate64 encoded data from next_in/avail_in, writing the decoded data to next_out/avail_out. The
* 'stream' MUST be in either the "initialized" state or the "Deflate64" state.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: should this get the same "This function returns..." as above (inflate)?

int result = 0;
while (stream.avail_in > 0)
for (std::size_t i = 0; size > 0; ++i)
{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a comment here about how you put it at the end of the buffer to make sure ASAN can detect reads off the end may not go unmissed

@dunhor dunhor merged commit 3b9c6c2 into main Jun 26, 2026
104 checks passed
@dunhor dunhor deleted the dunhor/oob_read branch June 26, 2026 23:59
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.

3 participants