Revert if calldata has wrong size

[chriseth]

An external function should revert in the following additional situations:

• it has statically-sized arguments and the calldata size does not exactly match that size
• it has dynamically-sized arguments and the calldata is shorter than the shortest possible encoding

[axic]

Related #510.

[federicobond]

I assume this has been recently motivated by potential attacks as described here:
https://blog.golemproject.net/how-to-find-10m-by-just-reading-blockchain-6ae9d39fcd95

Opt-in security like the one discussed in #510 is usually troublesome from my point of view, but with Solidity, it also costs quite a bit of money. How much overhead are we talking about if one would validate the calldata size for each function by default?

[federicobond]

On the other hand, I think there are good reasons to leave these checks to adequate tools for interfacing with contracts. No need to make every Ethereum node do more work to compensate for deficiencies in tooling.

[chriseth]

Something like require(msg.data.length == 32) costs roughly 25 gas. It will be more expensive to catch some errors for encoding dynamic data. Another thing to note is that, strictly, this would be a breaking change in the ABI.

[chriseth]

Can be done together with a rewrite of the ABI decoder.

[axic]

Raised over at Remix (https://github.com/ethereum/remix/issues/585) is a good suggestion that this should be definitely enforced for constructors, because it is unlikely case to create something without passing calldata. And considering it is concatenated after the bytecode it does leave a bigger surface for errors than regular transactions.

[axic]

This is kind of enabled now with the new ABI decoder. Keep in mind: turned the new ABI decoder on by default is a breaking change because of that.

[chriseth]

The old and the new decoder now have options to revert if the data is too short.

[lastperson]

it has statically-sized arguments and the calldata size does not exactly match that size

Will result in that all multisig wallets and other contracts that use address.call() will not be able to interact with newly compiled contracts. Details: #2884

[chriseth]

@lastperson we probably won't check for input being too long, only for being too short, so it should be fine.

The reason why we don't check for input being too long is that in order to do it properly (i.e. have a bijective decoding function), we also have to check whether there are no gaps between dynamically encoded data and that would be rather complicated.

[chriseth]

We actually have a check in place in the decoder, so it is easy to add this to 0.5.0. Do we want it?

[lastperson]

For "too short", I vote "yes".

[leonardoalt]

Closing after #4224 was merged.