Fix OOB write in BuildHuffmanTable.

First, BuildHuffmanTable is called to check if the data is valid.
If it is and the table is not big enough, more memory is allocated.

This will make sure that valid (but unoptimized because of unbalanced
codes) streams are still decodable.

Bug: chromium:1479274
Change-Id: I31c36dbf3aa78d35ecf38706b50464fd3d375741
This commit is contained in:
Vincent Rabaud
2023-09-07 21:16:03 +02:00
parent 7ba44f80f3
commit 902bc91903
4 changed files with 129 additions and 43 deletions

View File

@ -262,11 +262,11 @@ static int ReadHuffmanCodeLengths(
int symbol;
int max_symbol;
int prev_code_len = DEFAULT_CODE_LENGTH;
HuffmanCode table[1 << LENGTHS_TABLE_BITS];
HuffmanTables tables;
if (!VP8LBuildHuffmanTable(table, LENGTHS_TABLE_BITS,
code_length_code_lengths,
NUM_CODE_LENGTH_CODES)) {
if (!VP8LHuffmanTablesAllocate(1 << LENGTHS_TABLE_BITS, &tables) ||
!VP8LBuildHuffmanTable(&tables, LENGTHS_TABLE_BITS,
code_length_code_lengths, NUM_CODE_LENGTH_CODES)) {
goto End;
}
@ -286,7 +286,7 @@ static int ReadHuffmanCodeLengths(
int code_len;
if (max_symbol-- == 0) break;
VP8LFillBitWindow(br);
p = &table[VP8LPrefetchBits(br) & LENGTHS_TABLE_MASK];
p = &tables.curr_segment->start[VP8LPrefetchBits(br) & LENGTHS_TABLE_MASK];
VP8LSetBitPos(br, br->bit_pos_ + p->bits);
code_len = p->value;
if (code_len < kCodeLengthLiterals) {
@ -309,6 +309,7 @@ static int ReadHuffmanCodeLengths(
ok = 1;
End:
VP8LHuffmanTablesDeallocate(&tables);
if (!ok) return VP8LSetError(dec, VP8_STATUS_BITSTREAM_ERROR);
return ok;
}
@ -316,7 +317,8 @@ static int ReadHuffmanCodeLengths(
// 'code_lengths' is pre-allocated temporary buffer, used for creating Huffman
// tree.
static int ReadHuffmanCode(int alphabet_size, VP8LDecoder* const dec,
int* const code_lengths, HuffmanCode* const table) {
int* const code_lengths,
HuffmanTables* const table) {
int ok = 0;
int size = 0;
VP8LBitReader* const br = &dec->br_;
@ -367,8 +369,7 @@ static int ReadHuffmanCodes(VP8LDecoder* const dec, int xsize, int ysize,
VP8LMetadata* const hdr = &dec->hdr_;
uint32_t* huffman_image = NULL;
HTreeGroup* htree_groups = NULL;
HuffmanCode* huffman_tables = NULL;
HuffmanCode* huffman_table = NULL;
HuffmanTables* huffman_tables = &hdr->huffman_tables_;
int num_htree_groups = 1;
int num_htree_groups_max = 1;
const int max_alphabet_size =
@ -378,6 +379,10 @@ static int ReadHuffmanCodes(VP8LDecoder* const dec, int xsize, int ysize,
int* mapping = NULL;
int ok = 0;
// Check the table has been 0 initialized (through InitMetadata).
assert(huffman_tables->root.start == NULL);
assert(huffman_tables->curr_segment == NULL);
if (allow_recursion && VP8LReadBits(br, 1)) {
// use meta Huffman codes.
const int huffman_precision = VP8LReadBits(br, 3) + 2;
@ -429,16 +434,15 @@ static int ReadHuffmanCodes(VP8LDecoder* const dec, int xsize, int ysize,
code_lengths = (int*)WebPSafeCalloc((uint64_t)max_alphabet_size,
sizeof(*code_lengths));
huffman_tables = (HuffmanCode*)WebPSafeMalloc(num_htree_groups * table_size,
sizeof(*huffman_tables));
htree_groups = VP8LHtreeGroupsNew(num_htree_groups);
if (htree_groups == NULL || code_lengths == NULL || huffman_tables == NULL) {
if (htree_groups == NULL || code_lengths == NULL ||
!VP8LHuffmanTablesAllocate(num_htree_groups * table_size,
huffman_tables)) {
VP8LSetError(dec, VP8_STATUS_OUT_OF_MEMORY);
goto Error;
}
huffman_table = huffman_tables;
for (i = 0; i < num_htree_groups_max; ++i) {
// If the index "i" is unused in the Huffman image, just make sure the
// coefficients are valid but do not store them.
@ -463,19 +467,20 @@ static int ReadHuffmanCodes(VP8LDecoder* const dec, int xsize, int ysize,
int max_bits = 0;
for (j = 0; j < HUFFMAN_CODES_PER_META_CODE; ++j) {
int alphabet_size = kAlphabetSize[j];
htrees[j] = huffman_table;
if (j == 0 && color_cache_bits > 0) {
alphabet_size += (1 << color_cache_bits);
}
size = ReadHuffmanCode(alphabet_size, dec, code_lengths, huffman_table);
size =
ReadHuffmanCode(alphabet_size, dec, code_lengths, huffman_tables);
htrees[j] = huffman_tables->curr_segment->curr_table;
if (size == 0) {
goto Error;
}
if (is_trivial_literal && kLiteralMap[j] == 1) {
is_trivial_literal = (huffman_table->bits == 0);
is_trivial_literal = (htrees[j]->bits == 0);
}
total_size += huffman_table->bits;
huffman_table += size;
total_size += htrees[j]->bits;
huffman_tables->curr_segment->curr_table += size;
if (j <= ALPHA) {
int local_max_bits = code_lengths[0];
int k;
@ -510,14 +515,13 @@ static int ReadHuffmanCodes(VP8LDecoder* const dec, int xsize, int ysize,
hdr->huffman_image_ = huffman_image;
hdr->num_htree_groups_ = num_htree_groups;
hdr->htree_groups_ = htree_groups;
hdr->huffman_tables_ = huffman_tables;
Error:
WebPSafeFree(code_lengths);
WebPSafeFree(mapping);
if (!ok) {
WebPSafeFree(huffman_image);
WebPSafeFree(huffman_tables);
VP8LHuffmanTablesDeallocate(huffman_tables);
VP8LHtreeGroupsFree(htree_groups);
}
return ok;
@ -1352,7 +1356,7 @@ static void ClearMetadata(VP8LMetadata* const hdr) {
assert(hdr != NULL);
WebPSafeFree(hdr->huffman_image_);
WebPSafeFree(hdr->huffman_tables_);
VP8LHuffmanTablesDeallocate(&hdr->huffman_tables_);
VP8LHtreeGroupsFree(hdr->htree_groups_);
VP8LColorCacheClear(&hdr->color_cache_);
VP8LColorCacheClear(&hdr->saved_color_cache_);
@ -1666,7 +1670,7 @@ int VP8LDecodeImage(VP8LDecoder* const dec) {
if (dec == NULL) return 0;
assert(dec->hdr_.huffman_tables_ != NULL);
assert(dec->hdr_.huffman_tables_.root.start != NULL);
assert(dec->hdr_.htree_groups_ != NULL);
assert(dec->hdr_.num_htree_groups_ > 0);