Extra value in bids is silently kept by the auction contract

[dapperlabs]

Extra value in bids is silently kept by the auction contract

[Arachnid]

It's definitely safe to call msg.sender.transfer. They're the only ones who can cause that call to fail... in which case, why did they send the transaction in the first place?

[Arachnid]

Note this applies to SiringClockAuction, too.

[Arachnid]

Also note that fixing this will require modifying bidOnSiringAuction so KittyCore doesn't simply end up keeping excess funds in that case as well.

[dete]

Thanks for reporting this, @Arachnid!

This behaviour was actually intentional. Because of the nature of how our clock auction works, we anticipate that virtually every bid will have some small excess amount associated with it. However, we expected that in most cases, the gas cost of tracking and returning that excess would actually be higher than the amount in question. We figured that any non-trivial amount of excess would be something we could return manually.

It's worth pointing out that this logic was based on the assumption that the only safe way to return excess payment is to keep track of a withdrawal amount for each user, and include a withdrawal function, which is a ton of state. If it's safe to just call msg.sender.transfer(excess), that'd be a different story...

[kimcope]

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

Impact: Medium
Likelihood: Low
Points: 125

Please see the final leaderboard here.