fix(rar5): correct x86 E8/E9 filter range checks (corrupts real executables) - #121
Open
jdlien wants to merge 2 commits into
Open
fix(rar5): correct x86 E8/E9 filter range checks (corrupts real executables)#121jdlien wants to merge 2 commits into
jdlien wants to merge 2 commits into
Conversation
The decode-direction address fixup flattened unrar's nested condition
into an else-if:
if (addr < 0) { if (addr + off >= 0) addr += FILE_SIZE; }
else { if (addr < FILE_SIZE) addr -= off; }
A negative operand that stays negative after adding the position (a
byte pattern the encoder never rewrote, e.g. a stray 0xE8 inside a
ModRM/displacement sequence followed by high bytes) fell through into
the (addr - FILE_SIZE) check, which any negative value passes, and got
the position wrongly subtracted.
Found by differential testing against WinRAR 7.23 + UnRAR: the first
32 KiB of notepad.exe compressed with -ma5 -m3 (filter type 1, X86Call)
decoded to the right length but 48 wrong bytes across 15 sites, each
the 4-byte operand after a false-positive 0xE8; every wrong value was
exactly (original - position). With the nested checks the whole
corpus decodes byte-identical to unrar. Regression test carries the
verbatim case from that archive plus both boundary companions.
`{:#02x}` sets a width of 2 that the `#` (0x) alternate form makes
inapplicable, so current stable clippy fails the --all-features test
build under -D warnings. Use `{:#04x}` for the intended two-digit form.
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
1 participant
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.
The decode-direction x86 filter flattens unrar's nested sign check into an
else if, so a negative operand that stays negative after adding the position — a byte pattern the encoder never rewrote (e.g. a stray0xE8inside a ModRM/displacement sequence) — falls through and gets the position wrongly subtracted, corrupting real x86 code.Found by differential testing against WinRAR 7.23 + UnRAR: the first 32 KiB of notepad.exe compressed with
-ma5 -m3(filter type 1, X86Call) decoded to the right length but 48 wrong bytes across 15 sites, each the operand after a false-positive0xE8. Nesting the checks as unrar/libarchive do makes it decode byte-identical to unrar. Regression tests carry the verbatim case plus both boundary companions.Second commit is an unrelated one-liner:
tests/adc.rsuses{:#02x}, which current stable clippy rejects (unused_format_specs) under-D warnings, failing the--all-featurestest build. Folded in so CI is green here; happy to split it out if you'd prefer.