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

truster POC updated #3

Merged
merged 6 commits into from
Nov 16, 2024
Merged

truster POC updated #3

merged 6 commits into from
Nov 16, 2024

Conversation

igorganich
Copy link
Owner

No description provided.

Comment on lines +55 to +60
Then the opposite check will look like this:
```solidity
function _isSolved() private {
...
assert(token.balanceOf(address(pool)) != 0 || token.balanceOf(recovery) != TOKENS_IN_POOL);
}

Choose a reason for hiding this comment

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

neat and well explained so far 👌

Comment on lines +102 to +106
**0xaaaa0003** is the pool address. **0xab19e0c0** is the **flashLoan** function selector:
```javascript
$ cast 4b 0xab19e0c0
flashLoan(uint256,address,address,bytes)
```

Choose a reason for hiding this comment

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

cool thanks for adopting the cast 4b method

Fortunately, Halmos provides a special cheat-code for such cases: svm.createCalldata(). All we need to generate valid calldata is the contract type passed as a parameter to this cheat-code. One of the most obvious ways to use it in our attacker is this piece of code:
```solidity
contract SymbolicAttacker is Test, SymTest {
function attack() public {

Choose a reason for hiding this comment

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

Suggested change
function attack() public {
function attack() public {

Comment on lines 204 to 221
/*
** It is expected to receive a symbolic address as a parameter
** This function should return some concrete address and corresponding data.
** In the case of symbolic execution, the brute force over addresses
** is happening here!
*/
function get_concrete_from_symbolic (address /*symbolic*/ addr) public view
returns (address ret, bytes memory data)
{
for (uint256 i = 0; i < addresses_list_size; i++) {
if (addresses[i] == addr) {
string memory name = names_by_addr[addr];
return (addresses[i], svm.createCalldata(name));
}
}
revert(); // Ignore cases when addr is not some concrete known address
}
}

Choose a reason for hiding this comment

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

clever technique!

I would explain this slightly differently:

  • if addr is a concrete value, this returns (addr, symbolic calldata for addr)
  • if addr is symbolic, execution will split for each feasible case and it will return (addr0, symbolic calldata for addr0), (addr1, symbolic calldata for addr1), ..., and so on (one pair per path)
  • if addr is symbolic but has only 1 feasible value (e.g. with vm.assume(addr == ...)), then it should behave like the concrete case

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes, it would be much cleaner

}
```

We will not dwell on the implementation details of this contract. I will only say that this is a contract in which you can store address->contract name pairs. And also with its help you can conveniently brute force addresses symbolically. It is easier to show how to use it in practice. First, let's prepare Global Storage in Truster_Halmos.t.sol:

Choose a reason for hiding this comment

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

nit: can we rename the file TrusterHalmos.t.sol? let's drop the underscore, it's cleaner

// We can hardcode this address for convenience
GlobalStorage glob = GlobalStorage(address(0xaaaa0002));

function attack() public {

Choose a reason for hiding this comment

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

Suggested change
function attack() public {
function attack() public {

...
contract SymbolicAttacker is Test, SymTest {
// We can hardcode this address for convenience
GlobalStorage glob = GlobalStorage(address(0xaaaa0002));

Choose a reason for hiding this comment

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

can we pass it as a constructor parameter? hardcoding it seems brittle (for instance if we move things around a bit and addresses change, it would require debugging what's going on)

Copy link
Owner Author

@igorganich igorganich Nov 13, 2024

Choose a reason for hiding this comment

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

Of course, we normally expect to pass such contracts through the constructor. However, I did this specifically to emphasize that we can use this GlobalStorage from anywhere in the application. Imagine that we needed it in a contract generated by some factory contract. If we pass them through the constructor - we will have to rewrite each of these contracts by adding GlobalStorage passing logic. I thought it was less convenient than just hardcoding it where it was needed. And logging in at the very beginning immediately gives us the address of this contract.
What do you think?

Copy link
Owner Author

@igorganich igorganich Nov 13, 2024

Choose a reason for hiding this comment

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

This discussion will definitely end as soon as "native" storage appears in Halmos :). For example, Clang Static Analyzer also uses symbolic code execution and there is a possibility to use such storage to collect the necessary information https://clang.llvm.org/doxygen/ProgramStateTrait_8h.html#a6d1893bb8c18543337b6c363c1319fcf. How about an idea for improvement?

@igorganich igorganich merged commit 10dc18b into master Nov 16, 2024
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.

2 participants