CVE-2023-4863 AI-generated analysis
platform/external/webp, libwebp VP8L lossless decoder (src/dec/vp8l_dec.c, src/utils/huffman_utils.c)
The fixed-size HuffmanCode buffers in the VP8L (lossless WebP) decoder are replaced with a chained HuffmanTables segment structure. VP8LBuildHuffmanTable now does a dry run of BuildHuffmanTable to compute the exact table size and allocates a new segment when the current one is too small, so Huffman tables are no longer written past the allocated buffer.
Root cause
ReadHuffmanCodes in src/dec/vp8l_dec.c allocated one HuffmanCode array of num_htree_groups * table_size. It passed successive slices of that array to ReadHuffmanCode/VP8LBuildHuffmanTable, advancing by the size each call returned. ReadHuffmanCodeLengths used a stack array, HuffmanCode table[1 << LENGTHS_TABLE_BITS]. BuildHuffmanTable in src/utils/huffman_utils.c writes a root table plus second-level tables. Their total size depends on the code lengths in the bitstream: unbalanced codes produce more and larger second-level tables. BuildHuffmanTable wrote into root_table without checking the buffer size. The existing dry run with a NULL root_table could not be used to size the buffer, because the second-level loop did 'if (root_table == NULL) continue;'. According to the old header comment, a NULL root_table returned 0 or something greater than 0, not the table size. The commit title describes the result as an OOB write in BuildHuffmanTable.
How the fix works
- src/utils/huffman_utils.h: adds HuffmanTablesSegment {start, curr_table, next, size} and HuffmanTables {root, curr_segment}. It declares VP8LHuffmanTablesAllocate and VP8LHuffmanTablesDeallocate. VP8LBuildHuffmanTable now takes HuffmanTables* instead of HuffmanCode*. The comment saying a NULL root_table does not return the table size is removed.
- src/utils/huffman_utils.c:BuildHuffmanTable: the early 'if (root_table == NULL) continue;' is removed from the second-level loop. Table pointer advancement, the root_table[low] writes and ReplicateValue are each now guarded by 'if (root_table != NULL)'. table_bits, table_size, total_size, low and key are still updated in both modes, so the dry run now computes the total size including second-level tables.
- src/utils/huffman_utils.c:VP8LBuildHuffmanTable: first calls BuildHuffmanTable(NULL, ...) to validate the code and get total_size. It returns total_size if that is 0 or if root_table is NULL. Next it checks 'curr_segment->curr_table + total_size >= curr_segment->start + curr_segment->size'. If there is not enough room, it mallocs a new HuffmanTablesSegment of size max(total_size, current segment size), links it as curr_segment->next and makes it current. The rest of the old segment is left unused because the table must be contiguous. It then builds into curr_segment->curr_table, using a stack 'sorted' array when code_lengths_size <= SORTED_SIZE_CUTOFF and a heap array otherwise, and returns total_size.
- src/utils/huffman_utils.c: the new VP8LHuffmanTablesAllocate(size, tables) allocates the root segment with 'size' entries and sets curr_segment = &root. The new VP8LHuffmanTablesDeallocate frees root->start, sets start and next to NULL, then walks the chain and frees every following segment.
- src/dec/vp8li_dec.h: VP8LMetadata.huffman_tables_ changes from HuffmanCode* to an embedded HuffmanTables struct.
- src/dec/vp8l_dec.c:ReadHuffmanCodeLengths: the stack table is replaced by a HuffmanTables allocated with VP8LHuffmanTablesAllocate(1 << LENGTHS_TABLE_BITS) and built with VP8LBuildHuffmanTable. Lookups use tables.curr_segment->start[...], and VP8LHuffmanTablesDeallocate is called at End.
- src/dec/vp8l_dec.c:ReadHuffmanCode: the table parameter type changes to HuffmanTables*. The body is not shown in the material.
- src/dec/vp8l_dec.c:ReadHuffmanCodes: uses &hdr->huffman_tables_ directly and asserts it is zero-initialized. It allocates the tables with VP8LHuffmanTablesAllocate(num_htree_groups * table_size) inside the existing OOM check. After each ReadHuffmanCode it sets htrees[j] = huffman_tables->curr_segment->curr_table and advances curr_segment->curr_table by the returned size. On Error it calls VP8LHuffmanTablesDeallocate instead of WebPSafeFree.
- src/dec/vp8l_dec.c:ClearMetadata and VP8LDecodeImage: ClearMetadata now calls VP8LHuffmanTablesDeallocate, and VP8LDecodeImage now asserts hdr_.huffman_tables_.root.start != NULL.
- README.android: records the cherry-pick of 904941b4 and the merge of d64e6d7d.
Completeness
- The fix converts both callers of VP8LBuildHuffmanTable shown in the material. ReadHuffmanCodeLengths builds the code-length code table, which was formerly on the stack. ReadHuffmanCodes, through ReadHuffmanCode, builds the per-group tables in hdr->huffman_tables_.
- The size check sits inside VP8LBuildHuffmanTable and uses the total_size from the dry run, so it does not depend on the callers' size estimates. The original num_htree_groups * table_size allocation is kept only as the initial capacity.
- The check uses '>=' rather than '>'. This forces a new segment even when the table would exactly fill the remaining space, which is conservative.
- The fix does not tighten the initial capacity estimate. Valid unbalanced streams now succeed by allocating extra heap segments.
- Correctness relies on the returned total_size matching the number of entries written. The dry run and the real build share one code path.
- The commit touches only decoder and huffman_utils files, so encoder-side code is not changed by this commit.
Behavior changes
- Valid bitstreams with unbalanced Huffman codes whose tables exceed the initial capacity are now decoded. According to the commit message, this is intended to keep such streams decodable.
- There are more heap allocations. ReadHuffmanCodeLengths now mallocs a 1 << LENGTHS_TABLE_BITS table instead of using the stack, and extra HuffmanTablesSegment chunks are allocated on demand.
- If allocating a new segment fails, VP8LBuildHuffmanTable returns 0. In ReadHuffmanCodeLengths this ends in VP8_STATUS_BITSTREAM_ERROR.
- Each table build now runs BuildHuffmanTable twice, a dry run plus the real build.
- The VP8LMetadata layout changes because huffman_tables_ becomes an embedded struct. VP8LMetadata is defined in an internal decoder header.
Per major
- Android 14: Same fix. Google cherry-picked it as 9eac4f723bf7 into android-security-14.0.0_r2. README.android records 'cherry-pick 904941b4' on top of libwebp 1.3.0. At android-security-14.0.0_r29, 6/8 lines of the patched region are present and the HuffmanTables code is in place.
- Android 15: Not affected per the bulletin. At android-security-15.0.0_r17, 6/8 lines of the patched region are present.
- Android 16: Not affected per the bulletin. At android-security-16.0.0_r8, 6/8 lines of the patched region are present.
- Android 17: Not affected per the bulletin. At android-security-17.0.0_r1, 6/8 lines of the patched region are present.
Unknowns
- The material does not show the body of ReadHuffmanCode or how table_size is computed, so the original capacity estimate is not visible.
- Whether the replaced stack table in ReadHuffmanCodeLengths could itself overflow is not shown.
- Which Android processes or apps link the external/webp decoder is not shown.
- Whether 902bc919 and 2af26267 are upstream libwebp hashes or Android-internal ones is not shown.
- Why only 6/8 lines of the patched region match in later tags is not shown.