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

Possible kBrotliBitMask[n] overrun in BitMask()? #1196

Open
erAck opened this issue Sep 10, 2024 · 0 comments
Open

Possible kBrotliBitMask[n] overrun in BitMask()? #1196

erAck opened this issue Sep 10, 2024 · 0 comments

Comments

@erAck
Copy link

erAck commented Sep 10, 2024

May it be possible that BitMask(n) could get called with values n>32 so that at

return kBrotliBitMask[n];

kBrotliBitMask[n] would result in an out-of-bounds overrun?

For example, theoretically BrotliSafeReadBits32() and thus BrotliTakeBits() and BitMask() should never be called with n_bits > 32. The only call is from SafeReadBits32() where n_bits is passed from SafeProcessCommands() as b->dist_extra_bits[code], which is from the calculated meta-block in CalculateDistanceLut(). These values are never >32, hence the BROTLI_DCHECK(n_bits <= 32) assert. See also https://www.rfc-editor.org/rfc/rfc7932#section-4 "The number of extra bits can be 0..24".

However, it might be possible (did not check) to have a broken or prepare a crafted syntactically valid compressed file with a higher alphabet_size_limit value not matching ndirect and npostfix that results in dist_extra_bits array taking values >32.

As the BROTLI_DCHECK(n_bits <= 32) assert hits only in a debug build, the following BrotliTakeBits(br, n_bits, val) would be called with a higher n_bits value passed on to BitMask().

BitMask() is also used at numerous other places, a "safe" way would be to check n<=32 for kBrotliBitMask[n] there and return 0 if not, at the expense of a runtime performance penalty as this function is heavily used, or to handle it individually at places where it is called with calculated values (some places call it with constant safe values).

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

No branches or pull requests

1 participant