dapperlabs / cryptokitties-bounty

Bounty program for CryptoKitties smart contract
https://www.cryptokitties.co/
Other
482 stars 228 forks source link

Empty fallback in `ClockAuctionBase` has unintended consequences #3

Open Arachnid opened 6 years ago

Arachnid commented 6 years ago

Description

ClockAuctionBase:39 defines an empty fallback function. This overrides Solidity's default, which is a fallback function that always reverts.

Solidity's default already prohibits sending ether to the contract. By overriding the default with a function that does not throw or revert, this ensures calls to functions the contract does not implement will silently return instead of throwing.

Scenario

One example scenario is the tokenFallback of ERC223. This function is called on contracts when tokens are sent to them in order to avoid lost tokens, and it is expected that they throw if they do not want to accept tokens. This contract, by virtue of having an empty fallback, will silently accept (and trap) ERC223 token transfers.

Impact

Anyone attempting to call nonexistent functions on this contract will get a silently successful result with empty return data. Generally this is harmless, but in situations where someone is expecting the contract to implement a common interface, such as the scenario above, it may lead to lost funds.

Reproduction

See 'Scenario' above.

Fix

Remove the fallback function.

dete commented 6 years ago

Thanks for reporting this, @Arachnid! We'll be sure to take a look at this!

kimcope commented 6 years ago

Thanks for your participation, @Arachnid! Our team has reviewed your submission, and we are pleased to reward you for your report.

Impact: Low Likelihood: Low Points: 50

Please see the final leaderboard here.