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

Onchain fuzzing with custom setup #460

Closed
aviggiano opened this issue Apr 4, 2024 · 7 comments · Fixed by #461
Closed

Onchain fuzzing with custom setup #460

aviggiano opened this issue Apr 4, 2024 · 7 comments · Fixed by #461

Comments

@aviggiano
Copy link

Some attacks require the sender to have a previous balance of ERC20 tokens, for example. Usually, attackers get those through flash loans. A simpler way would be to just deal some tokens to the sender and see if it can extract value from a victim.

How can I test that with ityfuzz?

More specifically, I am trying to repro this with Ityfuzz:

https://blog.trailofbits.com/2023/07/21/fuzzing-on-chain-contracts-with-echidna/

@shouc
Copy link
Contributor

shouc commented Apr 4, 2024

Onchain flashloan can be enabled by adding ‘—flashloan’ flag. ItyFuzz would then automatically flashloan and liquidate related tokens from Uniswap, etc.

If the token has no LPs, you can use a Foundry fork test to set up (e.g., overwriting slots): https://github.com/fuzzland/ityfuzz/blob/master/tests/evm_manual/foundry1/test/Onchain.t.sol

runs with ‘ityfuzz evm -m OnchainTest — forge test’

@aviggiano
Copy link
Author

@shouc Not sure but it is not working.

I am using the following setup: https://github.com/aviggiano/ityfuzz/pull/1/files

ityfuzz/tests/evm_manual
[I] ➜ ityfuzz evm -m StaxExploitTest -- forge test
Nothing to compile
thread 'main' panicked at src/evm/mod.rs:612:54:
Failed to build the project: "no json file found"
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

jacob-chia added a commit that referenced this issue Apr 7, 2024
…stom-setup

fix #460: onchain-fuzzing-with-custom-setup
@jacob-chia
Copy link
Contributor

@aviggiano The issue has been fixed, with a few points to note:

  1. The test execution path is incorrect, it should be in tests/evm_manual/foundry1.
  2. The command ityfuzz evm -m StaxExploitTest -- forge test indicates the use of forge test to compile solidity files, but some tests under this path can only be tested via ityfuzz. Running forge test directly will fail. So forge build should be used instead.
  3. A etherscan_api_key is required to fetch code and ABI from Etherscan. The key can be obtained from Etherscan.

The complete steps are as follows:

git pull
cd tests/evm_manual/foundry1
cargo run evm -k <YOUR-SCAN-API-KEY> -m StaxExploitTest -- forge build

@aviggiano
Copy link
Author

Thank you @jacob-chia @shouc, this worked.
However, ityfuzz was not able to reproduce the bug that Echidna found in 10min.
Maybe something's wrong with my invariant test?

It would be nice to have some benchmarks for ityfuzz vs Echidna, which I believe are currently the only 2 fuzzers capable of fuzzing mainnet contracts. Please let me know if you want to work on this together.

@shouc shouc reopened this Apr 18, 2024
@shouc
Copy link
Contributor

shouc commented Apr 18, 2024

There is a regression bug making targetContract not picked up. Shall be fixed by EOD

@shouc
Copy link
Contributor

shouc commented Apr 18, 2024

Is there a reason that these lines are added?

        vm.prank(tokenHolder);
        StaxLP.transfer(address(this), initialAmount);
        StaxLP.approve(address(StaxLPStaking), type(uint256).max);

With them you just simply need to transfer to somewhere to break the invariant.

Removing these line shall yield the intended exploit with PR #469 :

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test} from "forge-std/Test.sol";

interface IStaxLP {
    function balanceOf(address) external returns (uint256);
    function transfer(address, uint256) external returns (bool);
    function approve(address, uint256) external returns (bool);
}

contract StaxExploitTest is Test {
    uint256 private initialAmount;
    IStaxLP private StaxLP =
        IStaxLP(0xBcB8b7FC9197fEDa75C101fA69d3211b5a30dCD9);
    address private StaxLPStaking = 0xd2869042E12a3506100af1D192b5b04D65137941;
    address private tokenHolder =
        address(0xeCb456EA5365865EbAb8a2661B0c503410e9B347);

    function setUp() public {
        vm.createSelectFork("http://64.71.166.16:28545/", 15725066);

        targetContract(address(StaxLP));
        targetContract(address(StaxLPStaking));

        initialAmount = StaxLP.balanceOf(address(this));
    }

    function invariant_1() public {
        assertEq(StaxLP.balanceOf(address(this)), initialAmount);
    }

}

Screen Shot 2024-04-18 at 12 25 35 PM

@aviggiano
Copy link
Author

@shouc well, these lines were on the original implementation from @tuturu-tech, but I guess they are not necessary after all.

Thanks for the fix in any case!

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 a pull request may close this issue.

4 participants