1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-27 14:07:45 +00:00

LibCompress: Fix off-by-one error in generate_huffman_lengths

Previously we would calculate the index of the first parent node as
heap.size() (which is initialized to non_zero_freqs), so in the edge
case in which all symbols had a non-zero frequency, we would use the
Size-index entry in the array for both the first symbol's leaf node,
and the first parent node.

The result would either be a non-optimal huffman code (bad), or an
illegal huffman code that would then go on to crash due to an error
check in CanonicalCode::from_bytes. (worse)

We now store parent nodes starting at heap.size() - 1, which eliminates
the potential overlap, and resolves the issue.
This commit is contained in:
Idan Horowitz 2023-12-01 20:42:54 +02:00 committed by Tim Schumacher
parent ec081a2ef5
commit b749167506
4 changed files with 21 additions and 3 deletions

View file

@ -12,3 +12,4 @@ foreach(source IN LISTS TEST_SOURCES)
endforeach() endforeach()
install(DIRECTORY brotli-test-files DESTINATION usr/Tests/LibCompress) install(DIRECTORY brotli-test-files DESTINATION usr/Tests/LibCompress)
install(DIRECTORY deflate-test-files DESTINATION usr/Tests/LibCompress)

View file

@ -11,8 +11,15 @@
#include <AK/MemoryStream.h> #include <AK/MemoryStream.h>
#include <AK/Random.h> #include <AK/Random.h>
#include <LibCompress/Deflate.h> #include <LibCompress/Deflate.h>
#include <LibCore/File.h>
#include <cstring> #include <cstring>
#ifdef AK_OS_SERENITY
# define TEST_INPUT(x) ("/usr/Tests/LibCompress/deflate-test-files/" x)
#else
# define TEST_INPUT(x) ("deflate-test-files/" x)
#endif
TEST_CASE(canonical_code_simple) TEST_CASE(canonical_code_simple)
{ {
Array<u8, 32> const code { Array<u8, 32> const code {
@ -155,3 +162,13 @@ TEST_CASE(deflate_compress_literals)
Array<u8, 0x13> test { 0, 0, 0, 0, 0x72, 0, 0, 0xee, 0, 0, 0, 0x26, 0, 0, 0, 0x28, 0, 0, 0x72 }; Array<u8, 0x13> test { 0, 0, 0, 0, 0x72, 0, 0, 0xee, 0, 0, 0, 0x26, 0, 0, 0, 0x28, 0, 0, 0x72 };
auto compressed = TRY_OR_FAIL(Compress::DeflateCompressor::compress_all(test, Compress::DeflateCompressor::CompressionLevel::GOOD)); auto compressed = TRY_OR_FAIL(Compress::DeflateCompressor::compress_all(test, Compress::DeflateCompressor::CompressionLevel::GOOD));
} }
TEST_CASE(ossfuzz_63183)
{
auto path = TEST_INPUT("clusterfuzz-testcase-minimized-FuzzDeflateCompression-6163230961303552.fuzz"sv);
auto test_file = MUST(Core::File::open(path, Core::File::OpenMode::Read));
auto test_data = MUST(test_file->read_until_eof());
auto compressed = TRY_OR_FAIL(Compress::DeflateCompressor::compress_all(test_data, Compress::DeflateCompressor::CompressionLevel::GOOD));
auto decompressed = TRY_OR_FAIL(Compress::DeflateDecompressor::decompress_all(compressed));
EXPECT(test_data == decompressed);
}

View file

@ -625,7 +625,7 @@ void DeflateCompressor::generate_huffman_lengths(Array<u8, Size>& lengths, Array
u16 heap_keys[Size]; // Used for O(n) heap construction u16 heap_keys[Size]; // Used for O(n) heap construction
u16 heap_values[Size]; u16 heap_values[Size];
u16 huffman_links[Size * 2 + 1] = { 0 }; u16 huffman_links[Size * 2] = { 0 };
size_t non_zero_freqs = 0; size_t non_zero_freqs = 0;
for (size_t i = 0; i < Size; i++) { for (size_t i = 0; i < Size; i++) {
auto frequency = frequencies[i]; auto frequency = frequencies[i];
@ -657,7 +657,7 @@ void DeflateCompressor::generate_huffman_lengths(Array<u8, Size>& lengths, Array
u16 second_lowest_frequency = heap.peek_min_key(); u16 second_lowest_frequency = heap.peek_min_key();
u16 second_lowest_link = heap.pop_min(); u16 second_lowest_link = heap.pop_min();
u16 new_link = heap.size() + 2; u16 new_link = heap.size() + 1;
heap.insert(lowest_frequency + second_lowest_frequency, new_link); heap.insert(lowest_frequency + second_lowest_frequency, new_link);
@ -676,7 +676,7 @@ void DeflateCompressor::generate_huffman_lengths(Array<u8, Size>& lengths, Array
non_zero_freqs++; non_zero_freqs++;
size_t bit_length = 1; size_t bit_length = 1;
while (link != 2) { while (link != 1) {
bit_length++; bit_length++;
link = huffman_links[link]; link = huffman_links[link];
} }