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

Concretize address in initLeaAst #962

Closed
wants to merge 3 commits into from

Conversation

PaDarochek
Copy link

Closes #956

@JonathanSalwan
Copy link
Owner

Can you explain in what scenarios triton::modes::CONCRETIZE_ADDRESS should be used instead of the AST evaluation ?

@SweetVishnya
Copy link
Contributor

AST evaluation may be wrong due to undertaint or missing instructions translations. We are getting real life examples for that. Computing memory address from concrete state allows us to avoid such errors. For instance:

mov eax, [ebx]

If ebx contains invalid AST, wrong memory will be dereferenced. While if we concretize ebx from concrete state, we dereference correct memory.

@JonathanSalwan
Copy link
Owner

JonathanSalwan commented Oct 28, 2020

We are getting real life examples for that.

Using a DBI or in pure emulation with Triton? (In pure emulation, concrete state is updated from AST evaluation).

Computing memory address from concrete state allows us to avoid such errors.

This is true only if you use a DBI to keep synch between the Triton state and the DBI state right?

Do you see any case where your patch must not be used as default behavior?

triton::uint64 concreteAddress = baseValue + indexValue * scaleValue + dispValue;

auto regSize = this->architecture->gprBitSize();
if (regSize != 64) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can remove the if and directly perform the &= whatever the regSize is equal to 64 or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is -Wshift-count-overflow and breaks my tests, because of O3 optimizations.

@SweetVishnya
Copy link
Contributor

Yeah, we use DBI, and this feature should be default. I updated the pull request.

@SweetVishnya
Copy link
Contributor

We solved this issue by synchronization of concrete and symbolic memory states. We compare concrete values received from DBI with Triton ASTs evaluation. If values are different, we concretize the corresponding register or memory. This also helps us to kill some symbolic registers after syscalls. So, now all memory accesses are correctly computed and there are no symbolic branches with wrong taken direction. @JonathanSalwan, you can close this PR and a corresponding issue #956.

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.

None yet

3 participants