echidna icon indicating copy to clipboard operation
echidna copied to clipboard

Config option to make all input address payable / able to receive ether

Open 0x6080 opened this issue 2 years ago • 8 comments

A common use-case is to involve paying ether to and from addresses involved in transactions. In such cases, a contract may require that the ether be able to be sent:

        (bool sent, ) = addr.call{value: msg.value}("");
        require(sent, "failed to send ether");

In fact, the compiler will give you a warning if you do not have this require check.

But in the current Echidna, some addresses can get used as inputs that are not able to receive Ether. I believe EOA's always can, so I think this means that a contract address is used and particularly a contract address that does not have a fallback function. I confirmed this hypothesis by doing this workaround in my test functions:

    function myMethod(address _addr) {
        bool hasFallback = testfallback(_addr);
        if (!hasFallback) {
            return;
        }
       // do normal call
    }
    // ...

    function testfallback(address _addr)
    private
    returns (bool _fallback)
    {
        bytes32 sig = bytes4(keccak256("fallback()"));
        bool _success = false;
        assembly {
            let x := mload(0x40)    // get free memory
            mstore(x, sig)          // store signature into it
            _success := call(
                5000,   // 5k gas
                _addr,  // to _addr
                0,      // 0 value
                x,      // input is x
                4,      // input length is 4
                x,      // store output to x
                32      // 1 output, 32 bytes
            )
            // _fallback is: _success && output
            _fallback := and(_success, mload(x))
        }
    }

Please let me know if there is a better workaround, since this seems hacky.

What would be perfect is if there was a config option for this. Something like onlyPayableAddresses: true when you want only addresses to be used as inputs that can receive ether.

0x6080 avatar Mar 12 '22 23:03 0x6080

Echidna will always simulate transactions originated from EOA, however, you can still use one (or many) proxy contracts to perform external call, and effectively restrict addresses that interact with the code you test (so, they will be always payable, with a fallback function)

ggrieco-tob avatar Mar 12 '22 23:03 ggrieco-tob

Echidna will always simulate transactions originated from EOA, however, you can still use one (or many) proxy contracts to perform external call, and effectively restrict addresses that interact with the code you test (so, they will be always payable, with a fallback function)

The random addresses are actually a really nice feature for my use-case though. It's just the random that can't receive ether that are the problem.

0x6080 avatar Mar 13 '22 00:03 0x6080

I think the cause here is that the input to some function should be restricted. For instance, If you have a function that should only take "payable" address, you can restrict its inputs to take only from such addressed. For instance, if you have a payTo, you can create a new one called payToPayable where you take the address from a dynamic array (payables):

function payTo(address x) public { .. }
function payToPayable(uint idx) public {
    idx = idx % payables.length; 
    payTo(payables[idx]);
 }

You can either keep the original payTo function or override it.

ggrieco-tob avatar Mar 14 '22 10:03 ggrieco-tob

I think the cause here is that the input to some function should be restricted. For instance, If you have a function that should only take "payable" address, you can restrict its inputs to take only from such addressed. For instance, if you have a payTo, you can create a new one called payToPayable where you take the address from a dynamic array (payables):

function payTo(address x) public { .. }
function payToPayable(uint idx) public {
    idx = idx % payables.length; 
    payTo(payables[idx]);
 }

You can either keep the original payTo function or override it.

I'm not sure if I understand. Let's say I have a function that Echidna generates inputs for:

    function myTestMethod(address input) {
       // ... need a way to return if input is not payable
       payTo(input);
    }

How would using the above payToPayable method protect this? Is payables an address array of only address that are payable? Isn't the whole point to have Echidna generate the inputs instead of hardcoding pre-defined ones?

0x6080 avatar Mar 15 '22 00:03 0x6080

How would using the above payToPayable method protect this?

payToPayable is just an example. You need to instrument myTestMethod in the same way.

Is payables an address array of only address that are payable?

Yes. You will have to add address that are either EOA, or contracts with fallback/receive suitable functions.

Isn't the whole point to have Echidna generate the inputs instead of hardcoding pre-defined ones?

Echidna will still be generating inputs (e.g. indexes to select an address), but you will apply a mapping to it instead of using it directly. If you absolutely require payable addresses as inputs, then this probably the best option.

Another approach is to discard unpayable addresses, using a try/catch statement, that checks if paying is possible or not (and then reverts).

ggrieco-tob avatar Mar 15 '22 07:03 ggrieco-tob

Well, that's essentially what I am doing in the original post, with testfallback(address _addr) skipping the test if the input address wasn't payable. I want to say this is probably the most efficient method but maybe I am wrong.

But still, I think having a config option for just only payable inputs would be desirable for many use-cases.

On second thought, does specifying the input as payable get this functionality automatically? ie: myMethod(address payable _addr) instead of myMethod(address _addr)?

0x6080 avatar Mar 19 '22 15:03 0x6080

Well, that's essentially what I am doing in the original post, with testfallback(address _addr) skipping the test if the input address wasn't payable. I want to say this is probably the most efficient method but maybe I am wrong.

But still, I think having a config option for just only payable inputs would be desirable for many use-cases.

I think this is already the most efficient approach. I think it won't work well if you try to implement this from the echidna code just because checking if an address does accept ETH transfer is not something we can do "statically" (unless it is an EOA, in that case it is trivial). Echidna will need to run a transaction to know if a address is payable or not, and will slow down a lot the fuzzing campaign.

On second thought, does specifying the input as payable get this functionality automatically? ie: myMethod(address payable _addr) instead of myMethod(address _addr)?

No. That is just a type you specify for the Solidity compiler in order to make your contract safe to interact.

ggrieco-tob avatar Mar 20 '22 09:03 ggrieco-tob

Well, that's essentially what I am doing in the original post, with testfallback(address _addr) skipping the test if the input address wasn't payable. I want to say this is probably the most efficient method but maybe I am wrong.

But still, I think having a config option for just only payable inputs would be desirable for many use-cases.

I think this is already the most efficient approach. I think it won't work well if you try to implement this from the echidna code just because checking if an address does accept ETH transfer is not something we can do "statically" (unless it is an EOA, in that case it is trivial). Echidna will need to run a transaction to know if a address is payable or not, and will slow down a lot the fuzzing campaign.

On second thought, does specifying the input as payable get this functionality automatically? ie: myMethod(address payable _addr) instead of myMethod(address _addr)?

No. That is just a type you specify for the Solidity compiler in order to make your contract safe to interact.

Maybe a config option for only EOA addresses if that's easier?

If not, I still think only payable addresses would be a useful abstraction, since the example I gave is a cumbersome assembly call.

0x6080 avatar Mar 20 '22 17:03 0x6080