code423n4 commented 1 year ago

Lines of code

https://github.com/ProjectOpenSea/seaport/blob/c30dd90272609677606f03f9c885466f15e891eb/contracts/lib/AmountDeriver.sol#L55

Vulnerability details

Impact

In the _locateCurrentAmount function, there is an assembly operation that is dividing by duration without a zero check, which could cause a division by zero error.

Proof of Concept

// Check for division by zero

    require(duration != 0, "Division by zero error"); 

    // Use mul() and add() function in solidity

    amount = (totalBeforeDivision + (roundUp ? 1 : 0)) / duration; 

    // Return the current amount.

    return amount;

} 

// Return the original amount as startAmount == endAmount.

return endAmount;

}

Tools Used

vs code

Recommended Mitigation Steps

mentioned in POC

0age commented 1 year ago

contested; this is implied by the following description in the natspec for this function, as this check has already been applied "upstream":

Note that this function expects that the startTime parameter of orderParameters is not greater than the current block timestamp and that the endTime parameter is greater than the current block timestamp. If this condition is not upheld, duration / elapsed / remaining variables will underflow.

HickupHH3 commented 1 year ago

Agreed. Function natspec explicitly mentions the conditions for the inputs.

c4-judge commented 1 year ago

HickupHH3 marked the issue as unsatisfactory: Insufficient proof

code-423n4 / 2023-01-opensea-findings