SBF-01M: Inexistent Sanitization of Sale Start

[JayWelsh]

SBF-01M: Inexistent Sanitization of Sale Start

Type	Severity	Location
Input Sanitization	Medium	SaleBuilderFacet.sol:L71, L113, L135, L227

Description:

The documentation states that the function should revert if the _start of the sale is in the past, however, no such logical guarantee is enforced in the code.

Example:

 * Reverts if:
 *  - Sale starts in the past
 *  - Sale exists for consignment
 *  - Consignment has already been marketed
 *
 * Emits a SalePending event.
 *
 * @param _consignmentId - id of the consignment being sold
 * @param _start - the scheduled start time of the sale
 * @param _price - the price of each item in the lot
 * @param _perTxCap - the maximum amount that can be bought in a single transaction
 * @param _audience - the initial audience that can participate. See {SeenTypes.Audience}
 */
function createPrimarySale (
    uint256 _consignmentId,
    uint256 _start,
    uint256 _price,
    uint256 _perTxCap,
    Audience _audience
)
external
override
onlyRole(SELLER)
onlyConsignor(_consignmentId)
{
    // Get Market Handler Storage slot
    MarketHandlerLib.MarketHandlerStorage storage mhs = MarketHandlerLib.marketHandlerStorage();

    // Fetch the consignment
    Consignment memory consignment = getMarketController().getConsignment(_consignmentId);

    // Make sure the consignment hasn't been marketed
    require(consignment.marketHandler == MarketHandler.Unhandled, "Consignment has already been marketed");

    // Get the storage location for the sale
    Sale storage sale = mhs.sales[consignment.id];

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

    // Set up the sale
    setAudience(_consignmentId, _audience);
    sale.consignmentId = _consignmentId;
    sale.start = _start;

Recommendation:

We advise such a require statement to be introduced to the codebase. If the _start is accidentally set to 0 in the current case, the NFT will be permanently locked.

[JayWelsh]

Docs modified to exclude the Sale starts in the past revert, it's alright if the start date is set to a past date (e.g. they might want it to start immediately), but adding a check for 0.

[JayWelsh]

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