Stop rejecting sparse CHDv5 images

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;

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

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

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 <stdio.h>
+#include <stdlib.h>
+
+#include <libchdr/chd.h>
+
+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;
+}