SRF-01M: Potential Consignment Vulnerability

[JayWelsh]

SRF-01M: Potential Consignment Vulnerability

Type	Severity	Location
Logical Fault	Major	SaleRunnerFacet.sol:L168-L196

Description:

The auction and sale systems are interfacing with the same consignment database, permitting an auction and sale for a particular consignment to run in parallel. This can lead to malicious pendingPayout claims for active auctions that also contain an active sale.

Example:

function claimPendingPayout(uint256 _consignmentId)
external
override
{
    // Get Market Handler Storage slot
    MarketHandlerLib.MarketHandlerStorage storage mhs = MarketHandlerLib.marketHandlerStorage();

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

    // Ensure that there is a pending payout
    require(consignment.pendingPayout > 0);

    // Ensure that caller is the seller
    require(consignment.seller == msg.sender);

    // Ensure that the sale has not yet sold out
    require((consignment.supply - consignment.releasedSupply) > 0, "Sale is sold out - call closeSale instead");

    // Make sure the sale exists and is running
    Sale storage sale = mhs.sales[_consignmentId];
    require(sale.start != 0, "Sale does not exist");
    require(sale.state == State.Running, "Sale hasn't started");

    // Distribute the funds (handles royalties, staking, multisig, and seller)
    disburseFunds(_consignmentId, consignment.pendingPayout);
    getMarketController().setConsignmentPendingPayout(consignment.id, 0);

}

Recommendation:

We advise this trait of the system to be prohibited by ensuring an auction and sale for the same consignment cannot run in parallel.

[JayWelsh]

Appeal:

We were under the impression that the marketConsignment function requirement of "consignment.marketHandler == MarketHandler.Unhandled" would prevent such a scenario, are we perhaps missing something here?

i.e. we thought this line here would have prevented this vuln:

seen-contracts/contracts/market/controller/facets/MarketClerkFacet.sol

Line 230 in 570abe0

require(consignment.marketHandler == MarketHandler.Unhandled, "Consignment has already been marketed");