Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty fallback in ClockAuctionBase has unintended consequences #3

Open
Arachnid opened this issue Nov 16, 2017 · 2 comments
Open

Empty fallback in ClockAuctionBase has unintended consequences #3

Arachnid opened this issue Nov 16, 2017 · 2 comments

Comments

@Arachnid
Copy link

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
Copy link

dete commented Nov 16, 2017

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

@kimcope
Copy link
Contributor

kimcope commented Nov 21, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants