search

coinbase / smart-wallet

MIT License
258 stars 47 forks source link

Optimize getting function selector #58

Closed wilsoncusack closed 2 months ago

wilsoncusack commented 2 months ago

https://github.com/cantinasec/review-coinbase-smartwallet/issues/4

xenoliss commented 2 months ago

There is a difference though, doing [0:4] will revert if msg.data is less than 4 bytes, while doing byte4(data) won't and simply right pad with zeros.

EDIT: Right padding is fine as the worst case is calling executeWithoutChainIdValidation() with no inner data. RE-EDIT: executeWithoutChainIdValidation() selector is 0x2c2abd1e so not reachable by right padding with zeros.

wilsoncusack commented 2 months ago

There is a difference though, doing [0:4] will revert if msg.data is less than 4 bytes, while doing byte4(data) won't and simply right pad with zeros.

Yes but this shouldn't cause any new security risk. If the function selector has trailing 0s, using the right padding to reach the same function selector should be functionally the same call flow.

wilsoncusack commented 2 months ago

Also @xenoliss I sanity checked the selector of executeWithoutChainIdValidation and it's 0x2c2abd1e, so this should never be possible.