Description

In the Marketplace contract, the _secondaryFee state variable is intended to be within the range [0, 10000].

However, the _setSecondarySaleFee() function does not have a check for this, which allows the fee to be set higher than the range.

This will cause the secondarySale() function to revert, as the following code will underflow:

uint256 fee = (name.price * _secondaryFee) / 10000;
totalFee += fee;
totalAmount += name.price;

uint256 sellerProfit = name.price - fee; // Zellic: underflow here

Impact

The impact is low, as this issue will only occur if the owner accidentally sets the secondary sale fee to a value higher than 10,000. Additionally, even if this does occur, it is easy to reset the fee back to a sane value.

Recommendations

We recommend adding checks to _setSecondarySaleFee() that ensure that the fee does not exceed 10,000.

A separate recommendation is to also consider a sane upper limit for the fee. For example, setting the fee to 10,000 does not make sense, because then the profit from the sale for the seller will be zero.

Remediation

The D3 Doma Team's position is as follows:

Won’t implement. Marketplace contract is deprecated, and is being replaced with Seaport. Since issue requires misconfiguration from admin side, and only results in reverts (not in funds loss), fix is not planned.