fix: three analyzer defects, and pin capstone below 6 - #239
Open
r0ny123 wants to merge 3 commits into
Open
Conversation
Capstone 6 renames the arm64 modules and constants to aarch64. It ships a compatibility shim for the old names, but the shim is partial: six of the twenty ARM64_* constants this codebase uses are missing from it, and it only takes effect once capstone.arm64 or capstone.arm64_const has been imported. Three modules import CS_ARCH_ARM64 straight from the capstone package before touching either submodule, so on capstone 6 they raise ImportError as soon as they load: smda/aarch64/AArch64Backend.py, smda/common/SmdaReport.py and smda/ida/IdaExporter.py. Reordering those imports is not enough on its own. With the shim loaded first, smda/aarch64/AArch64CapstoneVerification.py still fails on ARM64_OP_BARRIER, which capstone 6 no longer provides under that name. Moving to 6.x is a port rather than an import fix, so pin the dependency until someone does that port. Measured against capstone 6.0.0a10.
Two defects in the AArch64 recovery path, both about a register write the analyzer could not see. A relative jump table is signed whenever its entries can point backwards. The analyzer worked that out from an ldrsw, or from a standalone sxtw, but ARM64 also lets the compiler fold the sign-extension into the add that merges the index with the table base, as in "add x8, x8, w9, sxtw #2". Nothing else in that sequence marks the entries as signed, so the table was read as unsigned, every backward entry became a multi-gigabyte offset, and the walk aborted on the first one and lost the rest of the table. The add chase now reads the operand's extend field next to its shift. Separately, bl and blr overwrite x30, but capstone reports that write only in its implicit register list: bl's single operand is the branch target, and blr's operand is read rather than written. The constant tracker walks operands, so a value it had resolved into x30 survived a call and could still be handed to a later indirect-call or jump-table lookup. It now drops every register capstone marks as an implicit write, which covers both. One caveat for reviewers: the folded sign-extension is legal ARM64 but rare. It shows up zero times in 1105 branch-register sites across 669 system arm64 binaries, zero times in this repository's own AArch64 fixtures, and zero times in clang output at five optimisation levels for two targets. ldrsw is the usual choice there and was already handled. This is kept as a completeness path, not a measured recovery win. Instances: 2 files Validation: make lint, make test
popa and popad restore eax from the stack and carry no explicit operand, so the syscall-number backtrack treated them as instructions that touch nothing and kept walking past them. A "mov eax, N" from before the pop was then reported as the syscall number even though the pop had already replaced it. That is a wrong answer rather than an unresolved one, which is the worse of the two failures here. capstone spells the two forms popal and popaw, and lists eax among the registers they write implicitly. pushal and pushaw stay out of the set: they write only esp, so backtracking through them is correct. No bundled fixture output moves. The bundled 32-bit ELF has thirty-nine syscall sites and no popa anywhere, and the 32-bit dump that does contain a popa has no syscall sites. Instances: 1 file Validation: make lint, make test
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.
Three separate defects, all found while auditing the AArch64 backend against what capstone actually reports. Each one is a case of the analyzer not seeing a register write, and each has a test that fails without its fix.
What was wrong
A signed AArch64 jump table could be read as unsigned, and then thrown away. A relative jump table is signed when its entries are allowed to point backwards. The analyzer figured that out by spotting an
ldrsw, or a standalonesxtw. But ARM64 also lets the compiler fold that sign-extension into theaddthat merges the index with the table base —add x8, x8, w9, sxtw #2. When it does, nothing else in the sequence says the entries are signed. The table then gets read as unsigned, so the first entry pointing backwards turns into a multi-gigabyte offset, the walk gives up there, and the rest of the table is lost.A value tracked in x30 survived a call.
blandblrboth overwrite x30, but capstone only mentions that in its implicit register list:bl's single operand is the branch target, andblr's operand is read rather than written. The constant tracker walks operands, so it never saw either write. A value it had resolved into x30 could outlive the call and still be used by a later indirect-call or jump-table lookup.popahid the syscall number it had just overwritten.popaandpopadrestore eax from the stack and carry no explicit operand, so the syscall-number backtrack treated them as harmless and kept walking. Amov eax, Nfrom before the pop was then reported as the syscall number, even though the pop had already replaced it. That is a wrong answer rather than an unresolved one, which is the worse of the two.What changed
The jump-table add chase now reads the operand's extend field alongside its shift, so a folded
sxtb/sxth/sxtw/sxtxmarks the table as signed.The constant tracker now drops every register capstone reports as an implicit write. That covers
blandblrwithout an instruction-by-instruction list, and it cannot disturb anything the tracker resolved on purpose: of every mnemonic it models —adrp,adr,add,mov,movz,movk,ldr,ldur— none reports an implicit register write at all.popalandpopaw, which is how capstone spells those two forms, join the set of instructions known to write eax without saying so.pushalandpushawdeliberately stay out: they only write esp, so backtracking through them is correct.Separately, capstone is pinned below 6. Capstone 6 renames the arm64 modules and constants to aarch64, and while it ships a compatibility shim for the old names, that shim is partial — six of the twenty
ARM64_*constants used here are missing from it, and it only takes effect oncecapstone.arm64orcapstone.arm64_consthas been imported. Three modules importCS_ARCH_ARM64straight from the capstone package before touching either submodule, so they fail to import at all on capstone 6. Reordering those imports is not enough: with the shim loaded first, the AArch64 capstone-verification module still fails onARM64_OP_BARRIER. Moving to 6.x is a port, not an import fix, so the ceiling holds until someone does that port. Measured against capstone 6.0.0a10.How much this actually matters
Worth being straight about the first fix: the folded sign-extension is legal ARM64, but it is rare. It appears zero times in 1105 branch-register sites across 669 system arm64 binaries, zero times in this repository's own AArch64 fixtures, and zero times in clang output across five optimisation levels and two targets.
ldrswis what compilers actually emit there, and that path was already handled. This is kept as a completeness path for hand-written and obfuscated code, not as a measured recovery win — reviewers who would rather not carry speculative code should say so.The other two are ordinary correctness fixes with no such caveat.
No bundled fixture output moves for the
popachange either: the bundled 32-bit ELF has thirty-nine syscall sites and nopopaanywhere, and the 32-bit dump that does contain apopahas no syscall sites.Validation
make testmake lintmake typecheckdiff-cover coverage.xml --compare-branch=upstream/master --fail-under=100Five tests were added, and each was confirmed to fail before its fix rather than only passing after it. Neutralising the two AArch64 fixes fails three of the four new AArch64 tests; the fourth is a counter-case asserting that a plain
brdoes not invalidate anything, so it passes either way by design. Removingpopal/popawfrom the set makes the intel test return 1 — a falseexitsyscall — instead of nothing.