crytic / echidna

Ethereum smart contract fuzzer
https://secure-contracts.com/program-analysis/echidna/index.html
GNU Affero General Public License v3.0
2.75k stars 368 forks source link

VM failed for unhandled reason, Query <EVM.Query: fetch contract 0x0B1ba0af832d7C05fD64161E0Db78E85978E8082>. #818

Closed dxganta closed 1 year ago

dxganta commented 2 years ago

Hi I am getting this error though the contract address 0x0B1ba0af832d7C05fD64161E0Db78E85978E8082 is present on the init.json file. I am using etheno to pre-deploy the contracts.

I am using Echidna v2.0.2 and etheno version ToB/v0.3a1/source/Etheno

Screenshot 2022-08-25 at 2 13 41 PM Screenshot 2022-08-25 at 2 14 03 PM Screenshot 2022-08-25 at 2 37 01 PM
ggrieco-tob commented 2 years ago

Are you sure the deployment of 0x0B1ba0af832d7C05fD64161E0Db78E85978E8082 has not failed? (e.g reverted)

dxganta commented 2 years ago

@ggrieco-tob it did not revert I am sure of that. If the address is present in the init.json file then that means the contract was deployed successfully right?

ggrieco-tob commented 2 years ago

Not really. The JSON file only says that if the CREATE runs correctly, it will create a contract in such address. It is also possible that the execution runs correctly in the tests but failed to be reproduced inside Echidna. This could happen if the execution depends on the timestamp or some specific balance condition, for instance

dxganta commented 2 years ago

Is there any way/hack to check this? If the execution failed inside Echidna?

ggrieco-tob commented 2 years ago

Can we see the full code? It will be easier to give you an answer

dxganta commented 2 years ago

Yes sir. Here is the link to the full repo https://github.com/lindy-labs/sc_solidity-contracts/tree/echidna_tests

I am running yarn hardhat test test/echidna_init.spec.ts --network etheno to deploy the contracts on etheno.

and then run echidna-test . --contract Echidna_LiquityStrategy --config contracts/echidna/Echidna_LiquityStrategy.yaml to reproduce the bug.

also running etheno with the following params etheno --ganache --ganache-args "--gasLimit=0x1fffffffffffff --chain.allowUnlimitedContractSize -e 1000000000" -x ./init.json --debug

ggrieco-tob commented 2 years ago

It seems that part of your tests/deployment scripts rely on timestamps. Are these set correctly to run in Echidna?

dxganta commented 2 years ago

Can you show me which part if you dont mind please?

ggrieco-tob commented 2 years ago

https://github.com/lindy-labs/sc_solidity-contracts/search?q=time

dxganta commented 2 years ago

Yes but if you check time is not being used in the test/echidna_init.spec.ts file that I am using for deployment and neither in the deployment of the Vault.sol file where the error is occuring.

ggrieco-tob commented 2 years ago

Uhm, you are not using mainnet forking, right?

dxganta commented 2 years ago

nope. dropped that plan.

dxganta commented 2 years ago

I am pretty sure the problem is with the init.json file generated by etheno. Which version of etheno are we supposed to use with echidna v2.0.2?

ggrieco-tob commented 2 years ago

Perhaps could be the gas limit, which is very high. This needs to be debugged to see exactly what is failing to deploy and why. I will have some time for this but in around one week from today.

dxganta commented 2 years ago

It is not the version. I tried running the drizzle-box example and that ran fine. I also tried to run the test without that contract, but then it gave the same error for a different contract.

I will try changing the gas limit.

Yes sure take your time. Thank you.

dxganta commented 2 years ago

@ggrieco-tob The problem was with the proxy contract. When I commented out that line the tests ran fine without any issue.

Screenshot 2022-08-29 at 10 34 14 PM

This is an UUPS proxy and I am using hardhat's upgrades plugin to deploy the proxy contract.

ggrieco-tob commented 2 years ago

Interesting. The deployed proxy will have no ABI to call, but still good to know. We will try to reproduce it locally to make sure we can write some documentation. Thanks for your discussion!

dxganta commented 2 years ago

What do you think I should do to fix this? Use a non-proxy version of the contract for testing or is there some trick inside echidna ?

ggrieco-tob commented 2 years ago

Until I have some time to debug this, please use the non-proxy version (clearly, there is no need for that in the tests). If you want this issue to be fixed faster, please provide a minimal example where echidna fails to run when using the UUPS proxy (use a separated repository or a gist). In that case, we can investigate and fix it faster. :smiley:

dxganta commented 2 years ago

I changed the drizze-box example a little bit to reproduce this issue.

https://github.com/dxganta/drizzle-box This one is with a normal transparent proxy. https://github.com/dxganta/drizzle-box/tree/uups And this one is with the uups proxy

They dont exactly give out the same error that I am getting, but they fail weirdly without any message which doesn't happen if I dont use the proxy deployment.

ggrieco-tob commented 2 years ago

Thanks, I will take a look as soon I have some free time.

elopez commented 2 years ago

I gave the two drizzle-box branches you linked a try and they seem to work mostly fine. The only significant thing I noticed is that your E2E contract does not point to the proxy you are deploying:

https://github.com/dxganta/drizzle-box/blob/uups/contracts/crytic/E2E.sol#L8

e.g. in your uups branch, I believe it should point to this address instead:

https://github.com/dxganta/drizzle-box/blob/uups/.openzeppelin/unknown-1337.json#L29 https://github.com/dxganta/drizzle-box/blob/uups/init.json#L39 (address in "to", as "data" matches the signature for "set()")

could that be the issue you're seeing?

dxganta commented 2 years ago

@elopez Ahh I am sorry. That was a mistake on my part. But that was not the issue actually on my real repo. The real issue was these three methods which I was calling after deploying the proxy contract. Two of them are function calls to the strategy & vault contract.

Screenshot 2022-08-31 at 2 08 29 PM

https://github.com/lindy-labs/sc_solidity-contracts/blob/echidna_tests/test/echidna_init.spec.ts#L88-L92

ggrieco-tob commented 2 years ago

So you needed to modify these to fix this or you still have the issue and we must keep investigating?

dxganta commented 2 years ago

Yes I commented them out and the issue disappears. But any idea why the function calls caused the error?

elopez commented 2 years ago
    // await strategy.connect(admin).grantRole(MANAGER_ROLE, manager.address);

    // await vault.setStrategy(strategy.address);

Out of those three, the last two stand out to me; I don't see strategy being set to a value before this call. Did you mean to use strategyProxy?

dxganta commented 2 years ago

ahh I am sorry. missed a line in the comments there.

Screenshot 2022-08-31 at 5 42 02 PM

Here's the corrected one, uncommenting which still causes the issue.

https://github.com/lindy-labs/sc_solidity-contracts/blob/echidna_tests/test/echidna_init.spec.ts#L86-L92

rappie commented 2 years ago

I seem to be having a similar problem:

echidna-test . --contract E2E --config echidna-config.yaml --format text
Loaded total of 0 transactions from echidna-corpus/coverage
Analyzing contract: /home/rappie/Desktop/erc20-fuzzing/exercises/origindollar/fuzzing/contracts/E2E.sol:E2E
echidna-test: VM failed for unhandled reason, Query <EVM.Query: fetch contract 0x5409ED021D9299bf6814279A6A1411A7e866A631>. This shouldn't happen. Please file a ticket with this error message and steps to reproduce!

Etheno function call (if i remove this last function call from the init log it works)

  {
    "event": "FunctionCall",
    "from": "0x5409ed021d9299bf6814279a6a1411a7e866a631",
    "to": "0xb48e1b16829c7f5bd62b76cb878a6bb1c4625d7a",
    "gas_used": "0x324ff",
    "gas_price": "0x4a817c800",
    "data": "0x40c10f190000000000000000000000000b1ba0af832d7c05fd64161e0db78e85978e80820000000000000000000000000000000000000000000000056bc75e2d63100000",
    "value": "0x0"
  }

The line of code in my deployment:

await vault.connect(deployer).mint(dai.address, daiUnits("100"));

Interesting note: it does work when I do the mint in the E2E contract:

vault.mint(DAI_ADDRESS, 1000);

The project does contain proxies, but not for Vault.

Let me know if I can do anything to help. I can work around this for now by not minting in my deployment script. I might try narrowing things down later if I have the time.

elopez commented 2 years ago

Interesting, the error mentions EVM.Query: fetch contract 0x5409ED021D9299bf6814279A6A1411A7e866A631 but that's the same address as originating the function call (from) in your etheno call. Wouldn't that be an EOA?

ggrieco-tob commented 2 years ago

is the call to mint asking for the ETH balance?

rappie commented 2 years ago

Wouldn't that be an EOA?

Yes I noticed that aswell. It is indeed an EOA.

It still happens with a very minimal echidna config to it can't have anything to do with that:

initialize: etheno-log.json
multi-abi: true
testMode: assertion
corpusDir: echidna-corpus
rappie commented 2 years ago

is the call to mint asking for the ETH balance?

I dont think so, but i'm not 100% sure. Its actually from the Origin Dollar project ToB audited, so you might know the code better than me :smile:

https://github.com/OriginProtocol/origin-dollar/tree/d5dbffc4d80c2e8b5d0f6fd3f5f9a9cba6d6399c

   function mint(address _asset, uint256 _amount)
        external
        whenNotDepositPaused
    {
        require(assets[_asset].isSupported, "Asset is not supported");
        require(_amount > 0, "Amount must be greater than 0");

        uint256 price = IMinMaxOracle(priceProvider).priceMin(
            Helpers.getSymbol(_asset)
        );
        if (price > 1e8) {
            price = 1e8;
        }
        uint256 assetDecimals = Helpers.getDecimals(_asset);
        uint256 unitAdjustedDeposit = _amount.scaleBy(int8(18 - assetDecimals));
        uint256 priceAdjustedDeposit = _amount.mulTruncateScale(
            price.scaleBy(int8(10)), // 18-8 because oracles have 8 decimals precision
            10**assetDecimals
        );

        // Rebase must happen before any transfers occur.
        if (unitAdjustedDeposit >= rebaseThreshold && !rebasePaused) {
            rebase(true);
        }

        // Transfer the deposited coins to the vault
        IERC20 asset = IERC20(_asset);
        asset.safeTransferFrom(msg.sender, address(this), _amount);

        // Mint matching OUSD
        oUSD.mint(msg.sender, priceAdjustedDeposit);
        emit Mint(msg.sender, priceAdjustedDeposit);

        if (unitAdjustedDeposit >= autoAllocateThreshold) {
            allocate();
        }
    }
ggrieco-tob commented 2 years ago

@rappie can you do a quick debugging and comment code until you identify what line is causing the issue? It will be a lot faster to fix if we have that :smile:

ggrieco-tob commented 2 years ago

Oh, I think this is the issue:

    /**
     * @dev Is an accounts balance non rebasing, i.e. does not alter with rebases
     * @param _account Address of the account.
     */
    function _isNonRebasingAccount(address _account) internal returns (bool) {
        if (Address.isContract(_account)) {

That will trigger a query to an address that is not initialized

rappie commented 2 years ago

Yep that seems to be it. I've removed the checks and it works.

    function _isNonRebasingAccount(address _account) internal returns (bool) {
        return true;
    }
incertia commented 2 years ago

@ggrieco-tob do you think we should add a soft-fail option for contract fetches that just defaults to some "null contract"?

ggrieco-tob commented 2 years ago

We already have that during a fuzzing campaign, but not when the etheno transactions are replayed. In any case, I tried a quick fix here: https://github.com/crytic/echidna/pull/823 but I haven't tested the code, @rappie can you do it for us, please? :smiley:

rappie commented 2 years ago

Same problem :disappointed:

rappie commented 2 years ago

Here is a minimal etheno init file to reproduce the problem.

etheno-log.zip

ggrieco-tob commented 2 years ago

@rappie @dxganta please test #823 which contains a partial fix for this. More info available later.

rappie commented 2 years ago

It works :tada:

ggrieco-tob commented 1 year ago

Fixed in master