Skip to content

Conversation

daejunpark
Copy link
Collaborator

previously, all deployed contracts, including the test contract, were considered for calls to symbolic addresses. however, the test contract is typically not expected to be involved in such symbolic calls. moreover, recursively calling back to the test contract could lead to infinite looping behavior.

now, the test contract is excluded from symbolic calls to prevent this issue.

@daejunpark daejunpark requested a review from 0xkarmacoma October 7, 2024 19:29
@Elyx0
Copy link

Elyx0 commented Oct 9, 2024

I'm new to the symbolic logic, sometimes in hack reproductions we use the foundry test contract itself as callback receiver like https://github.com/SunWeb3Sec/DeFiHackLabs/blob/main/src/test/2024-09/Bankroll_exp.sol if it doesn't break that behavior that would be fine. Or make it clear that we have to deploy a specific Attacker contract in the setup beforehands to use symbolic functionality

@daejunpark
Copy link
Collaborator Author

I'm new to the symbolic logic, sometimes in hack reproductions we use the foundry test contract itself as callback receiver like https://github.com/SunWeb3Sec/DeFiHackLabs/blob/main/src/test/2024-09/Bankroll_exp.sol if it doesn't break that behavior that would be fine. Or make it clear that we have to deploy a specific Attacker contract in the setup beforehands to use symbolic functionality

@Elyx0 thanks for flagging! in your particular example, no addresses are symbolic, so this change won't have an impact. however, if you have a symbolic address, e.g., one passed as a test function parameter, or created using svm.createAddress() cheatcode, then you would need to deploy separate attacker contracts in setup(). otherwise, allowing callbacks to the test contract could lead to infinite loops.

we're merging this, but please let us know if it doesn't work for your use cases!

@daejunpark daejunpark merged commit 74a54fb into main Oct 11, 2024
31 checks passed
@daejunpark daejunpark deleted the fix/exclude-test-contract branch October 11, 2024 00:46
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