Verify the hunk CRC32 that v3/v4 map entries carry - #187
Merged
Conversation
Those entries hold a CRC32 of the decoded hunk and a flag to opt out of it. libchdr parsed the field and never used it, so on the pre-v5 path a hunk that decompressed cleanly to the wrong bytes was returned as valid data - the v5 path has checked its CRC16 all along. MAME rejects the same files with a decompression error. That gap also undercut the reasoning behind DR_FLAC_NO_CRC: dr_flac's own frame CRC is dropped because the hunk CRC is said to cover the same data, and on a v3/v4 AV hunk no hunk CRC was being checked at all. Covered by tests/legacy_decode.c against fixtures from tests/corpus/mklegacy.py, which builds the older layouts directly because chdman only writes v5. Every fixture agrees with MAME 0.264: the intact pair verifies, the two whose stored hunk no longer matches the advertised CRC are refused, and the one setting the opt-out flag reads again. Checked against real CHDv4 images too. The fixtures need only a Python interpreter, so the CMake job builds them; the test skips itself where they are absent, as parent-decode does. Costs about 1.1 KB of text on RV32 with bundled miniz, nearly all of it mz_crc32's table; nothing extra against a shared system zlib, and nothing when VERIFY_BLOCK_CRC is off. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CHDv3/v4 map entries hold a CRC32 of the decoded hunk plus a flag to opt out of it.
map_extract()has always parsed the field intoentry->crc, and nothing ever read it, so on the pre-v5 path a hunk that decompressed cleanly to the wrong bytes was handed back as valid data. The v5 path has checked its CRC16 all along. MAME rejects the same files with a decompression error.The gap also undercut the reasoning behind
DR_FLAC_NO_CRC: dr_flac's own frame CRC is dropped because the hunk CRC is said to cover the same data, andCHDCOMPRESSION_AVon a v3/v4 file routes through the samelibchdr_flac.cwith no hunk CRC being checked at all.The check mirrors MAME: COMPRESSED, UNCOMPRESSED and MINI entries, the
0x10opt-out honoured, self- and parent-referenced entries covered when the hunk they point at is read. v1/v2 entries have no room for a CRC andmap_extract_old()already sets the opt-out flag for them.Testing
tests/corpus/mklegacy.pybuilds the v3/v4 layouts directly, since chdman only writes v5. It emits an intact pair, two files whose stored hunk no longer matches the CRC the map advertises, and one that sets the opt-out flag on that same hunk. All ten cases agree with MAME 0.264 — the intact and opt-out files verify, the other two are refused.tests/legacy_decode.casserts that, and fails on master with fourgot "no error", want "decompression error".Also checked against three real CHDv4 images (76, 75 and 1817 hunks), which decode identically to
chdman extractraw, and against the v5 corpus plus a large real disc, whose output is unchanged. Built and run under the default configuration,CHDR_LOWRAM_TARGET=ON,WITH_SYSTEM_ZLIB=ONandCHDR_VERIFY_BLOCK_CRC=OFF.The fixtures need only a Python interpreter, unlike the CHDv5 corpus, so the CMake job builds them; the test skips itself where they are absent, as
parent-decodedoes.Not covered: no v3/v4 AVHuff fixture. The check sits after
decompress()regardless of codec, so AV is covered by construction, but nothing exercises it.Cost
About 1.1 KB of text on RV32 at
-Os, nearly all of itmz_crc32's table. Nothing extra against a shared system zlib, and nothing whenVERIFY_BLOCK_CRCis off.mz_crc32joins the ESP ROM rename list, since libchdr's own sources now name it.🤖 Generated with Claude Code