seen-haus / seen-contracts

Seen Haus contract suite
GNU General Public License v3.0
8 stars 2 forks source link

ABF-01M: Improper Sanitization of Start Time #8

Closed JayWelsh closed 2 years ago

JayWelsh commented 2 years ago

ABF-01M: Improper Sanitization of Start Time

Type Severity Location
Input Sanitization Major AuctionBuilderFacet.sol:L110, L115-L117

Description:

The logic of the code states that to make sure an auction doesn't exist, the auction.start member is validated to be zero as it is meant to always be non-zero on an actual auction. This is invalid as an auction with a _clock type equal to Trigger and a _start argument equal to 0 would still be considered as "inexistent" until the first bid comes in and thus be frozen in time.

Example:

// Make sure auction doesn't exist (start would always be non-zero on an actual auction)
require(auction.start == 0, "Auction exists");

// Make sure start time isn't in the past if the clock type is not trigger type
// It doesn't matter if the start is in the past if clock type is trigger type
// Because when the first bid comes in, that gets set to the start time anyway
if(_clock != Clock.Trigger) {
    require(_start >= block.timestamp, "Non-trigger clock type requires start time in future");
}

// Set up the auction
setAudience(_consignmentId, _audience);
auction.consignmentId = consignment.id;
auction.start = _start;
auction.duration = _duration;
auction.reserve = _reserve;
auction.clock = _clock;
auction.state = State.Pending;
auction.outcome = Outcome.Pending;

Recommendation:

We advise an else clause to be introduced to the _start sanitization that ensures the value is simply positive. This will solve all zero-based evaluations of an auction's existence, such as in AuctionRunnerFacet.

JayWelsh commented 2 years ago

Resolved by https://github.com/seen-haus/seen-contracts/pull/46