Auction DoS possibility still problematic

[adamkolar]

This comment in ClockAuctionBase:

            // NOTE: Doing a transfer() in the middle of a complex
            // method like this is generally discouraged because of
            // reentrancy attacks and DoS attacks if the seller is
            // a contract with an invalid fallback function. We explicitly
            // guard against reentrancy attacks by removing the auction
            // before calling transfer(), and the only thing the seller
            // can DoS is the sale of their own asset! (And if it's an
            // accident, they can call cancelAuction(). )
            seller.transfer(sellerProceeds);

correctly states that seller can only DoS sale of his own asset, but I think this is still problematic. It's true that contract that owns the kitty on sale can merely revert transaction by throwing, but this still imposes gas cost on buyer who attempted to place a valid bid. And since there is no easy way to discern good auctions from fake ones, a griefer can create a minefield which could hamper smooth trading. I think using withdraw pattern instead of immediate transfer is preferable.

[dete]

Boy, this is a tough one. You make a great point, but it's not easy weighing the possibility of a griefer (who would be burning their own gas for this) against the certainty of requiring ALL sellers to do a two-step withdrawal process (with guaranteed gas cost increase).

We'll discuss this internally, but I suspect we won't address this with a code change at the moment. If this becomes a problem, we do have the option to swap out the auction contract without having to change the core ownership contract. We also have a facility for cancelling any auction that is misbehaving.

I also suspect that most wallets would warn the user that the bid would fail before letting a user submit the transaction. It's certainly possible to construct a fallback that fails in practice, but which succeeds as a preview, but that further increases the cost of implementing this troll.

Thanks for reporting this! Even if we don't implement a code change today, we have something we can watch for.

[kimcope]

Thanks for your participation, @adamkolar!