Fix size_t overflow in encoder CreatePreparedDictionaryWithParams (32-bit)#1452
Fix size_t overflow in encoder CreatePreparedDictionaryWithParams (32-bit)#14520xazanul wants to merge 3 commits intogoogle:masterfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
On 32-bit platforms, sizeof(uint32_t) * source_size in the alloc_size computation can overflow size_t when source_size approaches 2^30 bytes. This causes a wrapped allocation size, leading to an undersized buffer and subsequent out-of-bounds writes. Split the allocation size computation into a fixed header_size and the variable source-dependent term, and add an overflow check before combining them.
391ac77 to
d07ae4d
Compare
|
Hi. Let's fix that on higher level. Dictionaries larger than ~64MiB are oxymōrum. With some generous amount forgiveness we can reject dictionaries larger than 256MiB (1<<28). So: in |
|
Thanks for the review! That makes sense capping at the protocol level is cleaner than guarding the arithmetic. I'll update the PR with: /* Brotli cannot reference dictionary data beyond 64 MiB (1 << 26). if (source_size > BROTLI_MAX_PREPARED_DICTIONARY_SIZE) return NULL; Will also double-check the exact distance limit from the spec to tighten the constant if appropriate. |
Per maintainer review: cap prepared dictionary source_size at 256 MiB (1 << 28) instead of guarding the allocation arithmetic. Dictionaries larger than ~64 MiB are beyond what Brotli can reference, so rejecting them at the protocol level is cleaner and sufficient.
d9486ec to
0eebdf1
Compare
|
Updated the PR with the changes you suggested:
Let me know if anything else needs adjusting! |
Summary
CreatePreparedDictionaryWithParams()(c/enc/compound_dictionary.c) wheresizeof(uint32_t) * source_sizewrapssize_ton 32-bit platforms whensource_sizeapproaches 2^30 bytesThe Bug
Lines 22–26 compute
alloc_sizeas a single expression:On 32-bit platforms (
size_t= 32 bits), the last term4 * source_sizeoverflows whensource_size >= 2^30(~1 GB). Whenalloc_sizewraps to a small value:BROTLI_ALLOCallocates an undersized bufferBROTLI_IS_NULLis a no-op in non-analyzer builds (#define BROTLI_IS_NULL(A) (!!0))slot_size,slot_limit,num,bucket_heads, andnext_bucketoverflow the heap bufferImpact
mallocreturns a valid pointer for the wrapped size, all subsequent writes go out of bounds — potential code execution on 32-bit targetsRequires a 32-bit platform and a large (~1 GB) dictionary passed to the encoder via
BrotliEncoderPrepareDictionary().The Fix
Split
alloc_sizeinto a fixedheader_size(slot/bucket tables) and the variablesource_size-dependent term. Before combining, check that the multiplication and addition do not overflowsize_t:Uses the existing
BROTLI_SIZE_MAXmacro frombrotli/types.h. ReturnsNULL(matching existing error-return convention) when the size would overflow.Note
This is distinct from the decoder-side
total_sizeoverflow fixed in #1450. That fix addressedAttachCompoundDictionary()inc/dec/decode.c; this fix addresses the encoder-side allocation inc/enc/compound_dictionary.c.Test plan
-Wall -Wextrasize_tis 64 bits)CreatePreparedDictionary()withsource_size >= 2^30returnsNULLinstead of overflowing