fix: buffer overflow in HUF_WriteCTableWksp for 256 symbols #4528
+72
−3
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes a buffer overflow bug in
HUF_WriteCTableWkspstructure that has existed since 2016. ThehuffWeightarray was incorrectly sized at 255 elements, but the code attempts to access index 255, causing an out-of-bounds write.Bug Details:
Location:
lib/compress/huf_compress.c:245Current (buggy) code:
Problem: At line 285, the code writes:
wksp->huffWeight[maxSymbolValue] = 0;When maxSymbolValue = 255 (valid for 256 unique symbols), this writes to huffWeight[255], which is out of bounds for a 255-element array.
Impact:
Current Limitation:
The bug is currently masked by a runtime check at line 282:
if (maxSymbolValue > (256-128)) return ERROR(GENERIC);This artificially limits the raw write path to 128 symbols, preventing the overflow but also preventing valid use of 256 symbols when FSE compression is not effective.
Real-World Impact:
This bug affects applications that:
Root Cause:
Introduced in commit 38b75dde (July 2016) during code simplification:
Intent: Remove special-case RLE handling for all-1 Huffman distributions
Side effect: Array size accidentally reduced from [HUF_SYMBOLVALUE_MAX + 1] (256) to [HUF_SYMBOLVALUE_MAX] (255)
Protection: Runtime check added simultaneously to prevent overflow, but it limits functionality
Fix:
Structure Fix:
Runtime Check Removal:
With the structure corrected, the artificial 128-symbol limit can be safely removed:
// REMOVED: if (maxSymbolValue > (256-128)) return ERROR(GENERIC);
Testing:
Added regression test tests/huf_max_symbol_test.c that:
Test Results:
Before fix:
Returns ERROR(GENERIC) due to runtime check blocking 256 symbols. After fix:
Successfully handles all 256 symbols.
Consistency:
Note that HUF_readCTable (line 294) already uses the correct size:
BYTE huffWeight[HUF_SYMBOLVALUE_MAX + 1]; // Already correct!
This fix brings HUF_WriteCTableWksp in line with the existing correct implementation.
Backward Compatibility
Checklist: