Should I validate msg.data.length for methods that may change storage?

http://securify.ch/ recommends this but I can't find any mentions/explanations elsewhere.

Shouldn't this only be a problem, when I typecast values, e.g. uint8 to uint256 or so?


This measure is to prevent the The ERC20 Short Address Attack. this attack occurs when an attacker use an incomplete address which ends with zeros 0,'00'...

if an attacked generate an address like 0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa0000. he can provide 0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa (zeros omitted) to the transfer function This would actually cause a transfer of a value shifted by 16 bits, i.e. 65536 times larger than X, to attacker’s Ethereum account.

the msg.data would be like following :

Function: transfer(address _to, uint256 _value)


to avoid that we have a 'malicious' msg.data we could check

 modifier onlyPayloadSize(uint numwords) {
        assert(msg.data.length == numwords * 32 + 4);

to read in depth about this attack please refer to the golem blog.


I've to add this check can cause other type of problems.

For example if you try to interact from a multisig wallet the data is padded to 32 bytes so a valid call will endup being rejected because of extra padding bytes were not taking into account, https://blog.coinfabrik.com/smart-contract-short-address-attack-mitigation-failure/.

Also if you want to implement inheritance between methods your msg.data can be less than the size you were expecting https://github.com/OpenZeppelin/zeppelin-solidity/issues/261. OpenZeppelin has recently removed all the checks https://github.com/OpenZeppelin/zeppelin-solidity/pull/277.

So the current recommendation is not to use that mitigation. You should not sign a transaction without validating the input paramters.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.