Unexpected fallback function behavior when returning values.

[Dexaran]

I have this contracts:

pragma solidity ^0.4.11;

contract test_receive
{ 
// this contract is needed only for the other contract to compile - it's never actually called
    function receive(address, uint256, bytes) returns (bool, uint256)
   { 
        return (false, 222);
    }
} 

contract test_cast
{
    uint public test_uint;
    bool public test_bool;

    function test(address addr, uint256 n) returns (uint256)
    {
        bytes memory _data;
        var (_success, _back) = test_receive(addr).receive(addr, n, _data);
        require(_success);
        test_uint = _back;
        test_bool = _success;
        return _back;
    }
}

contract dummy
  {
    function() payable {  }
}

I've compiled contracts and deployed test_cast at this address on Rinkeby: 0x330D0f8cc94758F26DD299bb803Fe63Fef76F726.

I've deployed dummy here on Rinkeby: 0x66855c513B861b617aD806C9d4CC35661087dda3

I've called test at test_cast with this parameters: 0x330d0f8cc94758f26dd299bb803fe63fef76f726 , 124 .

Call tx: https://rinkeby.etherscan.io/tx/0x88833366a0ca22e1aacdb64c7d2e3aefa54e7f719782e36a5fbde1a078c482cd

In the result of the call test_uint became 7477059611491291558618241337412310647290875865618134341354521175411004538880 and test_bool became true .

I think that if the fallback function doesn't return any values then it should not return values depending on my input.

[chriseth]

Solidity does not track invalid explicit type conversions at runtime. It is actually even impossible to detect them with the current ABI. After metropolis, such errors can be reduced but they can never be fully erased.

[Dexaran]

@chriseth I suppose the problem is not the explicit type conversion, but return value assignment.
I'm executing the test_receive(addr).receive(addr, n, _data); function assuming it will return values. There is no receive function at the executable contract and as the result fallback function is executed.

Fallback function must return nothing.

I think that if I've called a function that should not return values then my variables should stay unassigned (_success = false and _back = 0 in this case) or the transaction should end with execution error. Not to get returned random values from the stack.

But the return values were taken from the stack where my inputs variables were located I think.

[pirapira]

Before Metropolis, there is no way for the calling contract to see if the called account returned any data. The called contract can refuse to execute the fallback function when the input data is at least 4 bytes.

[axic]

The called contract can refuse to execute the fallback function when the input data is at least 4 bytes.

I think that may work better as a modifier (we could include it in std) or a keyword.

[Dexaran]

So we can't rely on variables returned by any third party contracts before Metropolis because they could be a random value from the stack, if I understand everything correctly.

[pirapira]

Yes, that's right. There are two cases before Metropolis:

• the memory content (in the memory region specified as output range) changes after CALL: the calling contract can be sure that something has been returned
• the memory content stays the same after CALL: the calling contract cannot be sure if it's the random leftover in the memory or something returned.

[MicahZoltu]

What is the problem with @pirapira's suggestion? If the caller is attempting to call a function that doesn't exist I think this SHOULD be an error/exception. The contract should not receive the function and instead execute unintended code and then return garbage as that is an incredibly unsafe default.

Even after Metropolis, I do not think that calling a non-existent function should ever do anything other than throw (including returning 0 values). I believe this can all be done in Solidity as the compiler just needs to make it so the default function checks the calldata to see if it is non-empty. If it is non-empty, then it should be assumed that a contract call was attempted and if the method lookup fails it should throw.

IMO, the default function should only be executed if the call is otherwise valid (no calldata or calldata is a valid function call).

[chriseth]

@MicahZoltu the called contract can detect whether the called function exists or not (at least unless there is a function selector collision), but earlier comments were about the caller detecting it. The latter is impossible because the EVM does not even have the concept of functions.

I support reverting if the function signature was not found, and only execute the fallback function in case of empty data.

[axic]

Since this can only be handled properly from the callee's side it only makes it to work in a non-malicious scenario. In those scenarios also using interface introspection is a possible way.

---

I think there are actually three kinds of fallback functions:
a) those which expect data (i.e. they are a proxy or use a non-ABI encoded input)
b) those which expect no data
c) those which expect that no other function selector was matched

Currently we support all options:

• c) is the catch all
• b) has a shortcut in the dispatcher and can be enforced via a modifier: modifier emptyCalldata() { require(msg.data.length == 0); _; }
• a) works because msg.data is accesible.

With this change we would break c) and a), which some contracts use.

I'd go with a way which can support all options and since it seems to be a bad idea to introduce fallbacks with different names (one of them would be function () and the other with a name?) the next best way is to have a modifier or a pragma.

These two older issues try to talk about a similar issue: #510 and #2109.

If we would like to make it a language feature then I'd suggest designing it together with these other issues because they tackle ABI decoding from different aspects:

• require zero length
• require exact length
• require minimum length

[MicahZoltu]

Hmm, good point about the existing use cases people have come up with for fallback functions. Delegation is an example one that is becoming pretty common in the ecosystem where the default function looks at the data and uses it even though there are no methods that match the called signature.

My concern is that I'm unconvinced these features are worth the risks they introduce. It is really unexpected behavior for anyone not previously familiar with it and at the moment the fact that the fallback returns garbage in the case of a non-match scares me a lot.

I would personally be OK with losing the auto-delegation feature (which I currently use pretty heavily) if it meant not having to worry about the default function being accidentally called and then possibly returning garbage data to me. I can't think of any other major languages that behave this way (where an invalid function call doesn't result in a hard failure), yet I haven't run into any problems where I thought to myself, "If only the call fell back to executing some other code I wrote...".

[3esmit]

Would be possible to clear return data in case of a fallback function that does not return anything? Like calling `assembly { return(0) }´... Maybe when fallback function starts executing it clear the returndata memory pointer and then do the logic. Maybe we could make a modifier ? Or make a modifier for when we need fallback function returning a unknown (that came from delegatecall)?

The only case I see that fallback returning any value is when we use it with delegatecall.