diff --git a/src/libchdr_chd.c b/src/libchdr_chd.c index b5981c2..6be528d 100644 --- a/src/libchdr_chd.c +++ b/src/libchdr_chd.c @@ -108,6 +108,10 @@ * (#147). */ #define CHD_MAX_FILE_SIZE (1024ULL * 1024 * 1024 * 1024) +/* Ceiling on a materialized CHDv5 hunk map. Not a format limit - it only + * bounds one allocation. LOWRAM_TARGET never materializes the map at all. */ +#define CHD_MAX_MAP_SIZE (256ULL * 1024 * 1024) + #define COOKIE_VALUE 0xbaadf00d #define END_OF_LIST_COOKIE "EndOfListCookie" @@ -861,10 +865,22 @@ static CHDR_INLINE int map_size_v5(chd_header* header, size_t *size) { /* Avoid overflow due to corrupted data. */ const size_t max_hunkcount = ((size_t)-1 / header->mapentrybytes); + uint64_t want; + if (header->hunkcount > max_hunkcount) return FALSE; - *size = (size_t)header->hunkcount * header->mapentrybytes; + /* A compressed v5 map is entropy-coded, so its on-disk size says nothing + * about how many hunks it holds and cannot be used to bound this. Cap the + * decompressed map directly: CHD_MAX_MAP_SIZE covers every image size + * this library claims to support (over 400GB at CD geometry, ~89GB at + * 4KB hunks) while keeping a malformed hunkcount from asking for more + * memory than any host would hand out. */ + want = (uint64_t)header->hunkcount * header->mapentrybytes; + if (want > CHD_MAX_MAP_SIZE) + return FALSE; + + *size = (size_t)want; return TRUE; } @@ -2945,13 +2961,25 @@ static chd_error header_read(chd_file *chd) if (header->hunkbytes >= CHD_MAX_HUNK_SIZE || ((uint64_t)header->hunkbytes * (uint64_t)header->totalhunks) >= CHD_MAX_FILE_SIZE) return CHDERR_INVALID_DATA; - /* totalhunks is used to size the map allocation; a malformed header - * can otherwise request multi-GB allocations for map[] even when the - * file itself is tiny. Every hunk map entry consumes at least one bit - * in the compressed on-disk map, so totalhunks cannot legitimately - * exceed file_size * 8. */ - if ((uint64_t)header->totalhunks > chd->file_size * 8) - return CHDERR_INVALID_DATA; + /* totalhunks sizes the map allocation, so a malformed header could + * otherwise ask for multi-GB of map[] against a tiny file. + * + * v1-v4 give an exact bound: their map is a plain array of fixed-size + * entries stored right after the header, so it has to fit in the file. + * v5 gets no equivalent - its map is run-length and Huffman coded, and a + * run of identical, uncompressed or self-referenced hunks costs far less + * than one bit each, so a valid sparse image really can carry far more + * hunks than its own size in bits. Bound what is actually allocated + * instead; see map_size_v5(). */ + if (header->version < 5) + { + uint64_t entrysize = (header->version < 3) ? OLD_MAP_ENTRY_SIZE : MAP_ENTRY_SIZE; + uint64_t mapend = (uint64_t)header->length + + (uint64_t)header->totalhunks * entrysize; + + if (mapend < (uint64_t)header->length || mapend > chd->file_size) + return CHDERR_INVALID_DATA; + } /* guess it worked */ return CHDERR_NONE; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8e6557b..f025e5d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -52,6 +52,15 @@ chdr_test_defines(chdr-legacy-decode) add_test(NAME legacy-decode COMMAND chdr-legacy-decode "${CMAKE_CURRENT_SOURCE_DIR}/corpus/legacy") +# A sparse CHDv5 whose map carries more hunks than the file has bits. Its own +# test rather than a seeds/ walk, because what is under test is chd_open() +# accepting it at all - a header check can reject it before any decode. +add_executable(chdr-sparse-map sparse_map.c) +target_link_libraries(chdr-sparse-map PRIVATE chdr-static) +chdr_test_defines(chdr-sparse-map) +add_test(NAME sparse-map + COMMAND chdr-sparse-map "${CMAKE_CURRENT_SOURCE_DIR}/corpus/seeds/rle_sparse.chd") + # CHDs with a parent: seeds/ has none, and cannot (a child needs its parent, and # the workflows there open every file standalone), so COMPRESSION_PARENT would # otherwise go untested - including through the in-place CD spread, where a diff --git a/tests/corpus/generate.sh b/tests/corpus/generate.sh index c9985b0..f2d83b6 100755 --- a/tests/corpus/generate.sh +++ b/tests/corpus/generate.sh @@ -82,6 +82,16 @@ create_hd hd_multi.chd -c zlib,lzma,huff,zstd # decoder holds the largest per-instance buffer of any codec here. create_hd hd_flac.chd -c flac +# A CHDv5 whose map compresses far below one bit per hunk: 64 MiB of zeros +# becomes a couple of hundred bytes carrying 16384 hunks, because a run of +# identical hunks costs almost nothing in the run-length and Huffman coded v5 +# map. Any header check that assumes a lower bound on map size per hunk +# rejects it - the regression case for issue #190. Its own file, since the +# shared RAW_HD is far too small to produce a run worth compressing. +dd if=/dev/zero of="$TMP/sparse.raw" bs=1M count=64 status=none +chdman createraw -f -o "$CORPUS/rle_sparse.chd" -i "$TMP/sparse.raw" \ + -hs 4096 -us 2048 -c zlib >/dev/null 2>&1 || true + # CD-ROM: default + per-codec. create_cd cd_default.chd create_cd cd_none.chd -c none diff --git a/tests/sparse_map.c b/tests/sparse_map.c new file mode 100644 index 0000000..414be56 --- /dev/null +++ b/tests/sparse_map.c @@ -0,0 +1,73 @@ +/* license:BSD-3-Clause + * copyright-holders:Romain Tisserand + * + * A CHDv5 map is run-length and Huffman coded, so a run of identical hunks + * costs far less than one bit each: 64 MiB of zeros compresses to a couple of + * hundred bytes carrying 16384 hunks. Header sanity checks that bound + * totalhunks by the file's own size in bits therefore reject perfectly valid + * sparse images - issue #190, where a 50 GB image of ~23 KB was refused. + * + * Skips when the corpus is absent, as the other corpus tests do. + */ +#include +#include + +#include + +int main(int argc, char **argv) +{ + const char *path = (argc > 1) ? argv[1] : "tests/corpus/seeds/rle_sparse.chd"; + chd_file *chd = NULL; + const chd_header *header; + unsigned char *buf; + chd_error err; + uint32_t i; + FILE *probe; + + probe = fopen(path, "rb"); + if (probe == NULL) + { + printf("sparse seed not generated (%s) - skipping\n", path); + printf("run tests/corpus/generate.sh to build it\n"); + return 0; + } + fclose(probe); + + err = chd_open(path, CHD_OPEN_READ, NULL, &chd); + if (err != CHDERR_NONE) + { + fprintf(stderr, "open failed: %s\n", chd_error_string(err)); + fprintf(stderr, "a sparse v5 map holds more hunks than the file has bits;" + " a header check must not assume otherwise\n"); + return 1; + } + + header = chd_get_header(chd); + printf("sparse map: %u hunks, %u hunkbytes\n", + (unsigned)header->totalhunks, (unsigned)header->hunkbytes); + + buf = (unsigned char *)malloc(header->hunkbytes); + if (buf == NULL) + { + chd_close(chd); + return 1; + } + + /* every hunk has to decode, not just the header parse */ + for (i = 0; i < header->totalhunks; i++) + { + err = chd_read(chd, i, buf); + if (err != CHDERR_NONE) + { + fprintf(stderr, "hunk %u: %s\n", i, chd_error_string(err)); + free(buf); + chd_close(chd); + return 1; + } + } + + printf("all %u hunks decoded\n", (unsigned)header->totalhunks); + free(buf); + chd_close(chd); + return 0; +}