From 96bdd36700b4fd43d4bbf578df994d37ee505955 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Fri, 25 Oct 2024 17:21:31 +1000 Subject: [PATCH 01/13] chd: Fix potential buffer overread in compressed data --- src/libchdr_chd.c | 93 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 72 insertions(+), 21 deletions(-) diff --git a/src/libchdr_chd.c b/src/libchdr_chd.c index 69d8a4f..a8578ac 100644 --- a/src/libchdr_chd.c +++ b/src/libchdr_chd.c @@ -712,22 +712,39 @@ static chd_error cdlz_codec_decompress(void *codec, const uint8_t *src, uint32_t { uint32_t framenum; cdlz_codec_data* cdlz = (cdlz_codec_data*)codec; + chd_error decomp_err; + uint32_t complen_base; /* determine header bytes */ - uint32_t frames = destlen / CD_FRAME_SIZE; - uint32_t complen_bytes = (destlen < 65536) ? 2 : 3; - uint32_t ecc_bytes = (frames + 7) / 8; - uint32_t header_bytes = ecc_bytes + complen_bytes; + const uint32_t frames = destlen / CD_FRAME_SIZE; + const uint32_t complen_bytes = (destlen < 65536) ? 2 : 3; + const uint32_t ecc_bytes = (frames + 7) / 8; + const uint32_t header_bytes = ecc_bytes + complen_bytes; + + /* input may be truncated, double-check */ + if (complen < (ecc_bytes + 2)) + return CHDERR_DECOMPRESSION_ERROR; /* extract compressed length of base */ - uint32_t complen_base = (src[ecc_bytes + 0] << 8) | src[ecc_bytes + 1]; + complen_base = (src[ecc_bytes + 0] << 8) | src[ecc_bytes + 1]; if (complen_bytes > 2) + { + if (complen < (ecc_bytes + 3)) + return CHDERR_DECOMPRESSION_ERROR; + complen_base = (complen_base << 8) | src[ecc_bytes + 2]; + } + if (complen < (header_bytes + complen_base)) + return CHDERR_DECOMPRESSION_ERROR; /* reset and decode */ - lzma_codec_decompress(&cdlz->base_decompressor, &src[header_bytes], complen_base, &cdlz->buffer[0], frames * CD_MAX_SECTOR_DATA); + decomp_err = lzma_codec_decompress(&cdlz->base_decompressor, &src[header_bytes], complen_base, &cdlz->buffer[0], frames * CD_MAX_SECTOR_DATA); + if (decomp_err != CHDERR_NONE) + return decomp_err; #ifdef WANT_SUBCODE - zlib_codec_decompress(&cdlz->subcode_decompressor, &src[header_bytes + complen_base], complen - complen_base - header_bytes, &cdlz->buffer[frames * CD_MAX_SECTOR_DATA], frames * CD_MAX_SUBCODE_DATA); + decomp_err = zlib_codec_decompress(&cdlz->subcode_decompressor, &src[header_bytes + complen_base], complen - complen_base - header_bytes, &cdlz->buffer[frames * CD_MAX_SECTOR_DATA], frames * CD_MAX_SUBCODE_DATA); + if (decomp_err != CHDERR_NONE) + return decomp_err; #endif /* reassemble the data */ @@ -795,22 +812,39 @@ static chd_error cdzl_codec_decompress(void *codec, const uint8_t *src, uint32_t { uint32_t framenum; cdzl_codec_data* cdzl = (cdzl_codec_data*)codec; + chd_error decomp_err; + uint32_t complen_base; /* determine header bytes */ - uint32_t frames = destlen / CD_FRAME_SIZE; - uint32_t complen_bytes = (destlen < 65536) ? 2 : 3; - uint32_t ecc_bytes = (frames + 7) / 8; - uint32_t header_bytes = ecc_bytes + complen_bytes; + const uint32_t frames = destlen / CD_FRAME_SIZE; + const uint32_t complen_bytes = (destlen < 65536) ? 2 : 3; + const uint32_t ecc_bytes = (frames + 7) / 8; + const uint32_t header_bytes = ecc_bytes + complen_bytes; + + /* input may be truncated, double-check */ + if (complen < (ecc_bytes + 2)) + return CHDERR_DECOMPRESSION_ERROR; /* extract compressed length of base */ - uint32_t complen_base = (src[ecc_bytes + 0] << 8) | src[ecc_bytes + 1]; + complen_base = (src[ecc_bytes + 0] << 8) | src[ecc_bytes + 1]; if (complen_bytes > 2) + { + if (complen < (ecc_bytes + 3)) + return CHDERR_DECOMPRESSION_ERROR; + complen_base = (complen_base << 8) | src[ecc_bytes + 2]; + } + if (complen < (header_bytes + complen_base)) + return CHDERR_DECOMPRESSION_ERROR; /* reset and decode */ - zlib_codec_decompress(&cdzl->base_decompressor, &src[header_bytes], complen_base, &cdzl->buffer[0], frames * CD_MAX_SECTOR_DATA); + decomp_err = zlib_codec_decompress(&cdzl->base_decompressor, &src[header_bytes], complen_base, &cdzl->buffer[0], frames * CD_MAX_SECTOR_DATA); + if (decomp_err != CHDERR_NONE) + return decomp_err; #ifdef WANT_SUBCODE - zlib_codec_decompress(&cdzl->subcode_decompressor, &src[header_bytes + complen_base], complen - complen_base - header_bytes, &cdzl->buffer[frames * CD_MAX_SECTOR_DATA], frames * CD_MAX_SUBCODE_DATA); + decomp_err = zlib_codec_decompress(&cdzl->subcode_decompressor, &src[header_bytes + complen_base], complen - complen_base - header_bytes, &cdzl->buffer[frames * CD_MAX_SECTOR_DATA], frames * CD_MAX_SUBCODE_DATA); + if (decomp_err != CHDERR_NONE) + return decomp_err; #endif /* reassemble the data */ @@ -1155,22 +1189,39 @@ static chd_error cdzs_codec_decompress(void *codec, const uint8_t *src, uint32_t { uint32_t framenum; cdzs_codec_data* cdzs = (cdzs_codec_data*)codec; + chd_error decomp_err; + uint32_t complen_base; /* determine header bytes */ - uint32_t frames = destlen / CD_FRAME_SIZE; - uint32_t complen_bytes = (destlen < 65536) ? 2 : 3; - uint32_t ecc_bytes = (frames + 7) / 8; - uint32_t header_bytes = ecc_bytes + complen_bytes; + const uint32_t frames = destlen / CD_FRAME_SIZE; + const uint32_t complen_bytes = (destlen < 65536) ? 2 : 3; + const uint32_t ecc_bytes = (frames + 7) / 8; + const uint32_t header_bytes = ecc_bytes + complen_bytes; + + /* input may be truncated, double-check */ + if (complen < (ecc_bytes + 2)) + return CHDERR_DECOMPRESSION_ERROR; /* extract compressed length of base */ - uint32_t complen_base = (src[ecc_bytes + 0] << 8) | src[ecc_bytes + 1]; + complen_base = (src[ecc_bytes + 0] << 8) | src[ecc_bytes + 1]; if (complen_bytes > 2) + { + if (complen < (ecc_bytes + 3)) + return CHDERR_DECOMPRESSION_ERROR; + complen_base = (complen_base << 8) | src[ecc_bytes + 2]; + } + if (complen < (header_bytes + complen_base)) + return CHDERR_DECOMPRESSION_ERROR; /* reset and decode */ - zstd_codec_decompress(&cdzs->base_decompressor, &src[header_bytes], complen_base, &cdzs->buffer[0], frames * CD_MAX_SECTOR_DATA); + decomp_err = zstd_codec_decompress(&cdzs->base_decompressor, &src[header_bytes], complen_base, &cdzs->buffer[0], frames * CD_MAX_SECTOR_DATA); + if (decomp_err != CHDERR_NONE) + return decomp_err; #ifdef WANT_SUBCODE - zstd_codec_decompress(&cdzs->subcode_decompressor, &src[header_bytes + complen_base], complen - complen_base - header_bytes, &cdzs->buffer[frames * CD_MAX_SECTOR_DATA], frames * CD_MAX_SUBCODE_DATA); + decomp_err = zstd_codec_decompress(&cdzs->subcode_decompressor, &src[header_bytes + complen_base], complen - complen_base - header_bytes, &cdzs->buffer[frames * CD_MAX_SECTOR_DATA], frames * CD_MAX_SUBCODE_DATA); + if (decomp_err != CHDERR_NONE) + return decomp_err; #endif /* reassemble the data */ From ebff5e7e1e5b61ea1de917d68d0682f13e9b523e Mon Sep 17 00:00:00 2001 From: Stenzek Date: Fri, 25 Oct 2024 17:40:02 +1000 Subject: [PATCH 02/13] chd: Avoid multiplication overflow in map_size_v5() --- src/libchdr_chd.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libchdr_chd.c b/src/libchdr_chd.c index a8578ac..8cf8562 100644 --- a/src/libchdr_chd.c +++ b/src/libchdr_chd.c @@ -41,6 +41,7 @@ #include #include #include +#include #include #include @@ -1542,6 +1543,11 @@ static inline void map_assemble(uint8_t *base, map_entry *entry) -------------------------------------------------*/ static inline int map_size_v5(chd_header* header) { + // Avoid overflow due to corrupted data. + const uint32_t max_hunkcount = (UINT32_MAX / header->mapentrybytes); + if (header->hunkcount > max_hunkcount) + return -1; + return header->hunkcount * header->mapentrybytes; } @@ -1628,6 +1634,8 @@ static chd_error decompress_v5_map(chd_file* chd, chd_header* header) enum huffman_error err; uint64_t curoffset; int rawmapsize = map_size_v5(header); + if (rawmapsize < 0) + return CHDERR_INVALID_FILE; if (!chd_compressed(header)) { From a154073a8a255f5c7704e343c2f9093eddbe2582 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Fri, 25 Oct 2024 18:03:39 +1000 Subject: [PATCH 03/13] chd: Fix potential buffer overflow in huffman_build_lookup_table() --- include/libchdr/huffman.h | 2 +- src/libchdr_huffman.c | 26 +++++++++++++++++++------- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/include/libchdr/huffman.h b/include/libchdr/huffman.h index 6c9f511..d771c29 100644 --- a/include/libchdr/huffman.h +++ b/include/libchdr/huffman.h @@ -85,6 +85,6 @@ int huffman_build_tree(struct huffman_decoder* decoder, uint32_t totaldata, uint enum huffman_error huffman_assign_canonical_codes(struct huffman_decoder* decoder); enum huffman_error huffman_compute_tree_from_histo(struct huffman_decoder* decoder); -void huffman_build_lookup_table(struct huffman_decoder* decoder); +enum huffman_error huffman_build_lookup_table(struct huffman_decoder* decoder); #endif diff --git a/src/libchdr_huffman.c b/src/libchdr_huffman.c index 556aa34..1a6d053 100644 --- a/src/libchdr_huffman.c +++ b/src/libchdr_huffman.c @@ -230,7 +230,9 @@ enum huffman_error huffman_import_tree_rle(struct huffman_decoder* decoder, stru return error; /* build the lookup table */ - huffman_build_lookup_table(decoder); + error = huffman_build_lookup_table(decoder); + if (error != HUFFERR_NONE) + return error; /* determine final input length and report errors */ return bitstream_overflow(bitbuf) ? HUFFERR_INPUT_BUFFER_TOO_SMALL : HUFFERR_NONE; @@ -272,7 +274,9 @@ enum huffman_error huffman_import_tree_huffman(struct huffman_decoder* decoder, error = huffman_assign_canonical_codes(smallhuff); if (error != HUFFERR_NONE) return error; - huffman_build_lookup_table(smallhuff); + error = huffman_build_lookup_table(smallhuff); + if (error != HUFFERR_NONE) + return error; /* determine the maximum length of an RLE count */ temp = decoder->numcodes - 9; @@ -308,7 +312,9 @@ enum huffman_error huffman_import_tree_huffman(struct huffman_decoder* decoder, return error; /* build the lookup table */ - huffman_build_lookup_table(decoder); + error = huffman_build_lookup_table(decoder); + if (error != HUFFERR_NONE) + return error; /* determine final input length and report errors */ return bitstream_overflow(bitbuf) ? HUFFERR_INPUT_BUFFER_TOO_SMALL : HUFFERR_NONE; @@ -523,8 +529,9 @@ enum huffman_error huffman_assign_canonical_codes(struct huffman_decoder* decode *------------------------------------------------- */ -void huffman_build_lookup_table(struct huffman_decoder* decoder) +enum huffman_error huffman_build_lookup_table(struct huffman_decoder* decoder) { + const lookup_value* lookupend = &decoder->lookup[(1u << decoder->maxbits)]; uint32_t curcode; /* iterate over all codes */ for (curcode = 0; curcode < decoder->numcodes; curcode++) @@ -533,9 +540,10 @@ void huffman_build_lookup_table(struct huffman_decoder* decoder) struct node_t* node = &decoder->huffnode[curcode]; if (node->numbits > 0) { - int shift; - lookup_value *dest; - lookup_value *destend; + int shift; + lookup_value *dest; + lookup_value *destend; + /* set up the entry */ lookup_value value = MAKE_LOOKUP(curcode, node->numbits); @@ -543,8 +551,12 @@ void huffman_build_lookup_table(struct huffman_decoder* decoder) shift = decoder->maxbits - node->numbits; dest = &decoder->lookup[node->bits << shift]; destend = &decoder->lookup[((node->bits + 1) << shift) - 1]; + if (dest >= lookupend || destend >= lookupend || destend < dest) + return HUFFERR_INTERNAL_INCONSISTENCY; while (dest <= destend) *dest++ = value; } } + + return HUFFERR_NONE; } From c1ed679fa35c11f25b313fe51fd717c01ce8f04b Mon Sep 17 00:00:00 2001 From: Stenzek Date: Fri, 25 Oct 2024 18:31:02 +1000 Subject: [PATCH 04/13] chd: Fix possible divide by zero in chd_get_metadata() --- src/libchdr_chd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libchdr_chd.c b/src/libchdr_chd.c index 8cf8562..306d837 100644 --- a/src/libchdr_chd.c +++ b/src/libchdr_chd.c @@ -2365,7 +2365,7 @@ CHD_EXPORT chd_error chd_get_metadata(chd_file *chd, uint32_t searchtag, uint32_ uint32_t faux_length; /* fill in the faux metadata */ - sprintf(faux_metadata, HARD_DISK_METADATA_FORMAT, chd->header.obsolete_cylinders, chd->header.obsolete_heads, chd->header.obsolete_sectors, chd->header.hunkbytes / chd->header.obsolete_hunksize); + sprintf(faux_metadata, HARD_DISK_METADATA_FORMAT, chd->header.obsolete_cylinders, chd->header.obsolete_heads, chd->header.obsolete_sectors, (chd->header.obsolete_hunksize != 0) ? (chd->header.hunkbytes / chd->header.obsolete_hunksize) : 0); faux_length = (uint32_t)strlen(faux_metadata) + 1; /* copy the metadata itself */ From 5572ace161bb9e6f827046d3e03a86a6a105af4b Mon Sep 17 00:00:00 2001 From: Stenzek Date: Fri, 25 Oct 2024 18:36:27 +1000 Subject: [PATCH 05/13] chd: Fix potential OOM in decompress_v5_map() --- src/libchdr_chd.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libchdr_chd.c b/src/libchdr_chd.c index 306d837..737315c 100644 --- a/src/libchdr_chd.c +++ b/src/libchdr_chd.c @@ -1632,13 +1632,17 @@ static chd_error decompress_v5_map(chd_file* chd, chd_header* header) uint8_t rawbuf[16]; struct huffman_decoder* decoder; enum huffman_error err; - uint64_t curoffset; + uint64_t curoffset; + const uint64_t file_size = core_fsize(chd->file); int rawmapsize = map_size_v5(header); if (rawmapsize < 0) return CHDERR_INVALID_FILE; if (!chd_compressed(header)) { + if ((header->mapoffset + rawmapsize) >= file_size || (header->mapoffset + rawmapsize) < header->mapoffset) + return CHDERR_INVALID_FILE; + header->rawmap = (uint8_t*)malloc(rawmapsize); if (header->rawmap == NULL) return CHDERR_OUT_OF_MEMORY; @@ -1658,6 +1662,8 @@ static chd_error decompress_v5_map(chd_file* chd, chd_header* header) parentbits = rawbuf[14]; /* now read the map */ + if ((header->mapoffset + mapbytes) < header->mapoffset || (header->mapoffset + mapbytes) >= file_size) + return CHDERR_INVALID_FILE; compressed_ptr = (uint8_t*)malloc(sizeof(uint8_t) * mapbytes); if (compressed_ptr == NULL) return CHDERR_OUT_OF_MEMORY; From 45670a9e0aa9669dce46be875a66b929caa11d9b Mon Sep 17 00:00:00 2001 From: Stenzek Date: Fri, 25 Oct 2024 18:58:51 +1000 Subject: [PATCH 06/13] chd: Don't try to initialize the same codec twice Prevents a double free on close. --- src/libchdr_chd.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/src/libchdr_chd.c b/src/libchdr_chd.c index 737315c..305656d 100644 --- a/src/libchdr_chd.c +++ b/src/libchdr_chd.c @@ -1953,7 +1953,8 @@ CHD_EXPORT chd_error chd_open_core_file(core_file *file, int mode, chd_file *par } else { - int decompnum; + int decompnum, needsinit; + /* verify the compression types and initialize the codecs */ for (decompnum = 0; decompnum < ARRAY_LENGTH(newchd->header.compression); decompnum++) { @@ -1970,8 +1971,21 @@ CHD_EXPORT chd_error chd_open_core_file(core_file *file, int mode, chd_file *par if (newchd->codecintf[decompnum] == NULL && newchd->header.compression[decompnum] != 0) EARLY_EXIT(err = CHDERR_UNSUPPORTED_FORMAT); + /* ensure we don't try to initialize the same codec twice */ + /* this is "normal" for chds where the user overrides the codecs, it'll have none repeated */ + needsinit = (newchd->codecintf[decompnum]->init != NULL); + for (i = 0; i < decompnum; i++) + { + if (newchd->codecintf[decompnum] == newchd->codecintf[i]) + { + /* already initialized */ + needsinit = 0; + break; + } + } + /* initialize the codec */ - if (newchd->codecintf[decompnum]->init != NULL) + if (needsinit) { void* codec = NULL; switch (newchd->header.compression[decompnum]) @@ -2131,10 +2145,24 @@ CHD_EXPORT void chd_close(chd_file *chd) for (i = 0 ; i < ARRAY_LENGTH(chd->codecintf); i++) { void* codec = NULL; + int j, needsfree; if (chd->codecintf[i] == NULL) continue; + /* only free each codec at max once */ + needsfree = 1; + for (j = 0; j < i; j++) + { + if (chd->codecintf[i] == chd->codecintf[j]) + { + needsfree = 0; + break; + } + } + if (!needsfree) + continue; + switch (chd->codecintf[i]->compression) { case CHD_CODEC_ZLIB: From 27476402fdc95d21eba1b99a832b3a3c79c4a804 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Fri, 25 Oct 2024 19:02:18 +1000 Subject: [PATCH 07/13] chd: Stop reading rawmap if EOF reached --- src/libchdr_chd.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/libchdr_chd.c b/src/libchdr_chd.c index 305656d..014486e 100644 --- a/src/libchdr_chd.c +++ b/src/libchdr_chd.c @@ -1703,7 +1703,16 @@ static chd_error decompress_v5_map(chd_file* chd, chd_header* header) rawmap[0] = lastcomp, repcount--; else { - uint8_t val = huffman_decode_one(decoder, bitbuf); + uint8_t val; + if (bitstream_overflow(bitbuf)) + { + free(compressed_ptr); + free(bitbuf); + delete_huffman_decoder(decoder); + return CHDERR_DECOMPRESSION_ERROR; + } + + val = huffman_decode_one(decoder, bitbuf); if (val == COMPRESSION_RLE_SMALL) rawmap[0] = lastcomp, repcount = 2 + huffman_decode_one(decoder, bitbuf); else if (val == COMPRESSION_RLE_LARGE) From 62e41f380a6e9f14dee52e05633073353eba7d05 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Fri, 25 Oct 2024 19:21:39 +1000 Subject: [PATCH 08/13] chd: Store overall file size and bounds check precached reads --- src/libchdr_chd.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/libchdr_chd.c b/src/libchdr_chd.c index 014486e..882d12b 100644 --- a/src/libchdr_chd.c +++ b/src/libchdr_chd.c @@ -299,6 +299,7 @@ struct _chd_file uint32_t cookie; /* cookie, should equal COOKIE_VALUE */ core_file * file; /* handle to the open core file */ + uint64_t file_size; /* size of the core file */ chd_header header; /* header, extracted from file */ chd_file * parent; /* pointer to parent file, or NULL */ @@ -1633,14 +1634,13 @@ static chd_error decompress_v5_map(chd_file* chd, chd_header* header) struct huffman_decoder* decoder; enum huffman_error err; uint64_t curoffset; - const uint64_t file_size = core_fsize(chd->file); int rawmapsize = map_size_v5(header); if (rawmapsize < 0) return CHDERR_INVALID_FILE; if (!chd_compressed(header)) { - if ((header->mapoffset + rawmapsize) >= file_size || (header->mapoffset + rawmapsize) < header->mapoffset) + if ((header->mapoffset + rawmapsize) >= chd->file_size || (header->mapoffset + rawmapsize) < header->mapoffset) return CHDERR_INVALID_FILE; header->rawmap = (uint8_t*)malloc(rawmapsize); @@ -1662,7 +1662,7 @@ static chd_error decompress_v5_map(chd_file* chd, chd_header* header) parentbits = rawbuf[14]; /* now read the map */ - if ((header->mapoffset + mapbytes) < header->mapoffset || (header->mapoffset + mapbytes) >= file_size) + if ((header->mapoffset + mapbytes) < header->mapoffset || (header->mapoffset + mapbytes) >= chd->file_size) return CHDERR_INVALID_FILE; compressed_ptr = (uint8_t*)malloc(sizeof(uint8_t) * mapbytes); if (compressed_ptr == NULL) @@ -1862,6 +1862,9 @@ CHD_EXPORT chd_error chd_open_core_file(core_file *file, int mode, chd_file *par newchd->cookie = COOKIE_VALUE; newchd->parent = parent; newchd->file = file; + newchd->file_size = core_fsize(file); + if ((int64_t)newchd->file_size <= 0) + EARLY_EXIT(err = CHDERR_INVALID_FILE); /* now attempt to read the header */ err = header_read(newchd, &newchd->header); @@ -2064,19 +2067,15 @@ CHD_EXPORT chd_error chd_open_core_file(core_file *file, int mode, chd_file *par CHD_EXPORT chd_error chd_precache(chd_file *chd) { int64_t count; - uint64_t size; if (chd->file_cache == NULL) { - size = core_fsize(chd->file); - if ((int64_t)size <= 0) - return CHDERR_INVALID_DATA; - chd->file_cache = malloc(size); + chd->file_cache = malloc(chd->file_size); if (chd->file_cache == NULL) return CHDERR_OUT_OF_MEMORY; core_fseek(chd->file, 0, SEEK_SET); - count = core_fread(chd->file, chd->file_cache, size); - if (count != size) + count = core_fread(chd->file, chd->file_cache, chd->file_size); + if (count != chd->file_size) { free(chd->file_cache); chd->file_cache = NULL; @@ -2723,7 +2722,10 @@ static uint8_t* hunk_read_compressed(chd_file *chd, uint64_t offset, size_t size #endif if (chd->file_cache != NULL) { - return chd->file_cache + offset; + if ((offset + size) > chd->file_size || (offset + size) < offset) + return NULL; + else + return chd->file_cache + offset; } else { @@ -2749,6 +2751,9 @@ static chd_error hunk_read_uncompressed(chd_file *chd, uint64_t offset, size_t s #endif if (chd->file_cache != NULL) { + if ((offset + size) > chd->file_size || (offset + size) < offset) + return CHDERR_READ_ERROR; + memcpy(dest, chd->file_cache + offset, size); } else @@ -3091,7 +3096,7 @@ static chd_error map_read(chd_file *chd) } /* verify the length */ - if (maxoffset > core_fsize(chd->file)) + if (maxoffset > chd->file_size) { err = CHDERR_INVALID_FILE; goto cleanup; From 6259d92990d938419620b972813e138869b274a2 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Fri, 25 Oct 2024 19:25:30 +1000 Subject: [PATCH 09/13] chd: Fix potential buffer overread in hunk_read_compressed() If the caller tries to read more than a hunk's worth of data. --- src/libchdr_chd.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libchdr_chd.c b/src/libchdr_chd.c index 882d12b..07304be 100644 --- a/src/libchdr_chd.c +++ b/src/libchdr_chd.c @@ -2729,6 +2729,10 @@ static uint8_t* hunk_read_compressed(chd_file *chd, uint64_t offset, size_t size } else { + /* make sure it isn't larger than the compressed buffer */ + if (size > chd->header.hunkbytes) + return NULL; + core_fseek(chd->file, offset, SEEK_SET); bytes = core_fread(chd->file, chd->compressed, size); if (bytes != size) From bc0690bbe608e43cd6d6ced13314857ca400f629 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Fri, 25 Oct 2024 19:28:44 +1000 Subject: [PATCH 10/13] chd: Fix potential memory leak in huffman_import_tree_huffman() --- src/libchdr_huffman.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libchdr_huffman.c b/src/libchdr_huffman.c index 1a6d053..2332104 100644 --- a/src/libchdr_huffman.c +++ b/src/libchdr_huffman.c @@ -273,10 +273,16 @@ enum huffman_error huffman_import_tree_huffman(struct huffman_decoder* decoder, /* then regenerate the tree */ error = huffman_assign_canonical_codes(smallhuff); if (error != HUFFERR_NONE) + { + delete_huffman_decoder(smallhuff); return error; + } error = huffman_build_lookup_table(smallhuff); if (error != HUFFERR_NONE) + { + delete_huffman_decoder(smallhuff); return error; + } /* determine the maximum length of an RLE count */ temp = decoder->numcodes - 9; From b2e05f546c746570ab7476fac05fd06518e369fa Mon Sep 17 00:00:00 2001 From: Stenzek Date: Fri, 25 Oct 2024 19:37:06 +1000 Subject: [PATCH 11/13] chd: Enforce some basic size limits in header_validate() Prevents going OOM/swapping/etc on files with corrupted headers. --- src/libchdr_chd.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/libchdr_chd.c b/src/libchdr_chd.c index 07304be..5cb04aa 100644 --- a/src/libchdr_chd.c +++ b/src/libchdr_chd.c @@ -89,6 +89,11 @@ #define CHD_V1_SECTOR_SIZE 512 /* size of a "sector" in the V1 header */ +#define CHD_MAX_HUNK_SIZE (128 * 1024 * 1024) /* hunk size probably shouldn't be more than 128MB */ + +/* we're currently only using this for CD/DVDs, if we end up with more than 10GB data, it's probably invalid */ +#define CHD_MAX_FILE_SIZE (10ULL * 1024 * 1024 * 1024) + #define COOKIE_VALUE 0xbaadf00d #define MAX_ZLIB_ALLOCS 64 @@ -2529,6 +2534,10 @@ static chd_error header_validate(const chd_header *header) return CHDERR_INVALID_PARAMETER; } + /* some basic size checks to prevent huge mallocs */ + if (header->hunkbytes >= CHD_MAX_HUNK_SIZE || ((uint64_t)header->hunkbytes * (uint64_t)header->totalhunks) >= CHD_MAX_FILE_SIZE) + return CHDERR_INVALID_PARAMETER; + return CHDERR_NONE; } From 70cfcbe98d811c4547e98cdff92c32261b0d7ec9 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Fri, 25 Oct 2024 20:42:20 +1000 Subject: [PATCH 12/13] cmake: Add libfuzzer harness and target --- CMakeLists.txt | 8 +++++ tests/CMakeLists.txt | 10 ++++++ tests/fuzz.c | 80 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+) create mode 100644 tests/fuzz.c diff --git a/CMakeLists.txt b/CMakeLists.txt index a9625d4..6afa8d7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -17,6 +17,14 @@ if(BUILD_LTO) endif() endif() +option(BUILD_FUZZER "Build instrumented binary for fuzzing with libfuzzer, requires clang") +if(BUILD_FUZZER) + # Override CFLAGS early for instrumentation. Disable shared libs for instrumentation. + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address,fuzzer-no-link") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address,fuzzer-no-link") + set(BUILD_SHARED_LIBS OFF) +endif() + include(GNUInstallDirs) #-------------------------------------------------- diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f100483..b4da3ed 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,2 +1,12 @@ add_executable(chdr-benchmark benchmark.c) target_link_libraries(chdr-benchmark PRIVATE chdr-static) + +# fuzzing +if(BUILD_FUZZER) + add_executable(chdr-fuzz fuzz.c) + target_link_options(chdr-fuzz PRIVATE "-fsanitize=address,fuzzer") + target_link_libraries(chdr-fuzz PRIVATE chdr-static) + add_custom_target(fuzz + COMMAND "$" "-max_len=131072" + DEPENDS chdr-fuzz) +endif() diff --git a/tests/fuzz.c b/tests/fuzz.c new file mode 100644 index 0000000..42a84e8 --- /dev/null +++ b/tests/fuzz.c @@ -0,0 +1,80 @@ +#include +#include +#include +#include +#include + +typedef struct { + const uint8_t *buffer; + size_t buffer_size; + size_t buffer_pos; +} membuf; + +static uint64_t membuf_fsize(struct chd_core_file *cf) { + return ((membuf *)cf->argp)->buffer_size; +} + +static size_t membuf_fread(void *buf, size_t size, size_t count, + struct chd_core_file *cf) { + membuf *mb = (membuf *)cf->argp; + if ((UINT32_MAX / size) < count) + return 0; + size_t copy = size * count; + size_t remain = mb->buffer_size - mb->buffer_pos; + if (remain < copy) + copy = remain; + memcpy(buf, &mb->buffer[mb->buffer_pos], copy); + mb->buffer_pos += copy; + return copy; +} + +static int membuf_fclose(struct chd_core_file *cf) { return 0; } + +static int membuf_fseek(struct chd_core_file *cf, int64_t pos, int origin) { + membuf *mb = (membuf *)cf->argp; + if (origin == SEEK_SET) { + if (pos < 0 || (size_t)pos > mb->buffer_size) + return -1; + mb->buffer_pos = (size_t)pos; + return 0; + } else if (origin == SEEK_CUR) { + if (pos < 0 && (size_t)-pos > mb->buffer_pos) + return -1; + else if ((mb->buffer_pos + (size_t)pos) > mb->buffer_size) + return -1; + mb->buffer_pos = + (pos < 0) ? (mb->buffer_pos - (size_t)-pos) : (mb->buffer_pos + pos); + return 0; + } else if (origin == SEEK_END) { + mb->buffer_pos = mb->buffer_size; + return 0; + } else { + return -1; + } +} + +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { + unsigned int i; + unsigned int totalbytes; + void *buffer; + membuf mb = {data, size, 0u}; + struct chd_core_file cf = {&mb, membuf_fsize, membuf_fread, membuf_fclose, + membuf_fseek}; + chd_file *file; + const chd_header *header; + chd_error err = chd_open_core_file(&cf, CHD_OPEN_READ, NULL, &file); + if (err != CHDERR_NONE) + return 0; + + header = chd_get_header(file); + totalbytes = header->hunkbytes * header->totalhunks; + buffer = malloc(header->hunkbytes); + for (i = 0; i < header->totalhunks; i++) { + err = chd_read(file, i, buffer); + if (err != CHDERR_NONE) + continue; + } + free(buffer); + chd_close(file); + return 0; +} From 09fa975c7763972a7e3205d83ea0f18104278728 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Sat, 2 Nov 2024 22:24:25 +1000 Subject: [PATCH 13/13] chd: Check that metadata seek succeeds Prevents an infinite loop if the offset is invalid. --- src/libchdr_chd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libchdr_chd.c b/src/libchdr_chd.c index 5cb04aa..19e9c4d 100644 --- a/src/libchdr_chd.c +++ b/src/libchdr_chd.c @@ -3144,7 +3144,8 @@ static chd_error metadata_find_entry(chd_file *chd, uint32_t metatag, uint32_t m uint32_t count; /* read the raw header */ - core_fseek(chd->file, metaentry->offset, SEEK_SET); + if (core_fseek(chd->file, metaentry->offset, SEEK_SET) != 0) + break; count = core_fread(chd->file, raw_meta_header, sizeof(raw_meta_header)); if (count != sizeof(raw_meta_header)) break;