util/chd: fix several size and range handling issues - #15893
Merged
Conversation
Check the full hunk byte-by-byte when determining whether it is zero-filled. The previous code checked the buffer as uint32_t values, which could leave the final 1-3 bytes unchecked when the hunk size was not divisible by four. Checking bytes also avoids requiring the input buffer to be aligned for uint32_t access. CHATGPT 6.3 used to help find what to change.
Reject zero hunk and unit sizes before calculating the hunk and unit counts. This prevents division by zero when opening a malformed CHD.
Reject zero hunk and unit sizes before checking the number of units per hunk. This prevents modulo by zero when creating a CHD with invalid size parameters.
Treat zero-length byte reads and writes as no-ops. This avoids unsigned underflow when calculating the last affected hunk from offset + bytes - 1.
Keep parent unit references in 64-bit values when compressing V5 maps. Also perform parent unit index calculations in 64-bit arithmetic to avoid truncation or overflow for references beyond the 32-bit range.
simzy39
force-pushed
the
chd-modernize-2
branch
from
August 14, 2026 04:18
4689746 to
d6d1369
Compare
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
2 participants
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.
Fix several CHD correctness and robustness issues:
Check every byte when detecting zero-filled hunks. The previous code
interpreted the buffer as 32-bit words and examined
m_hunkbytes / 4elements. If the hunk size was not divisible by four, this left the
final one to three bytes unchecked and could incorrectly treat a hunk
with non-zero trailing data as all zero.
Reject zero hunk and unit sizes when reading V5 headers. These values
are subsequently used as divisors when calculating the hunk and unit
counts, so accepting zero values could result in division by zero for
a malformed CHD.
Validate hunk and unit sizes when creating CHDs. Reject zero values
before performing the units-per-hunk modulo calculation, and reject
hunk sizes that are not an integral number of units.
Treat zero-length 'read_bytes' and 'write_bytes' operations as
successful no-ops. Without the early return, the calculation of the
last affected hunk uses 'offset + bytes - 1', which underflows when
'bytes' is zero.
Preserve 64-bit parent unit references throughout parent-map
handling. Parent-map item numbers are already represented as
'uint64_t', but storing a lookup result in a 32-bit value could
truncate a large parent unit number. In addition, the expression
used to construct parent unit numbers was evaluated using 32-bit
arithmetic before being passed to the 64-bit map. Promote the hunk
number before multiplication so the calculation itself is performed
in 64-bit arithmetic.
These changes do not alter the CHD file format.
AI assistance: ChatGPT (GPT-5.6 Sol) was used to help audit the code,
identify edge cases, to understand it, to propose changes, and review the proposed changes.
I've manually made the changes and have reviewed them.