Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

makes the disassembler more strict #1381

Merged
merged 1 commit into from
Dec 6, 2021

Conversation

ivg
Copy link
Member

@ivg ivg commented Dec 3, 2021

In #1375 instead of terminating the program on a knowledge base conflict during instruction lifting, we decided to treat such instruction as invalid and retract it and the whole path that led to it from the set of valid instructions. It turned out that the retraction mechanism wasn't quite complete and there were certain cases when an invalid instruction was still reachable, which triggered conflicts downstream, e.g., during the CFG reconstruction.

The problem mostly arises in the interworked code, where we have to guess whether an instruction is in A32 or T32 mode using heuristics such as byte patterns, which inevitably leads to conflicts. So the first place where we have to enfore agreement is in the encoding detection. Before this change, the information provided by the knowledge base had precedence over the natural rules of encodings, i.e., that the fall or regular jump can't change the encoding, unless it is the encoding changing jump.

In addition, whenever we discover a fall or a jump to an already disassembled instruction we have to check if the encodings agree and discard it if they don't.

Finally, there were some missing cases, when the invalid code wasn't retracted. First of all, it was possible when a jump destination was invalid but the jump remained in the code set. And the dual problem, when a basic block entry point was canceled not all incoming destinations were canceled - only the path through which the block was discovered. The last two issues were fixed and they affect even those targets that do not use interworking, e.g., x86. Which is good, as more code is discarded as invalid and gives us better CFG.

In BinaryAnalysisPlatform#1375 instead of terminating the program on a knowledge base
conflict during instruction lifting, we decided to treat such instruction
as invalid and retract it and the whole path that led to it from the
set of valid instructions. It turned out that the retraction mechanism
wasn't quite complete and there were certain cases when an invalid
instruction was still reachable, which triggered conflicts downstream,
e.g., during the CFG reconstruction.

The problem mostly arises in the interworked code, where we have to
guess whether an instruction is in A32 or T32 mode using heuristics
such as byte patterns, which inevitably leads to conflicts. So the
first place where we have to enfore agreement is in the encoding
detection. Before this change, the information provided by the
knowledge base had precedence over the natural rules of encodings,
i.e., that the fall or regular jump can't change the encoding,
unless it is the encoding changing jump.

In addition, whenever we discover a fall or a jump to an already
disassembled instruction we have to check if the encodings agree and
discard it if they don't.

Finally, there were some missing cases, when the invalid code wasn't
retracted. First of all, it was possible when a jump destination was
invalid but the jump remained in the code set. And the dual problem,
when a basic block entry point was canceled not all incomming
destinations were canceled - only the path through which the block was
discovered. The last two issues were fixed and they affect even those
targets that do not use interworking, e.g., x86. Which is good, as
more code is discarded as invalid and gives us better CFG.
@ivg ivg merged commit dceac1e into BinaryAnalysisPlatform:master Dec 6, 2021
@ivg ivg deleted the harden-driver branch December 6, 2021 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant