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

0 stars 0 forks source link

_locateCurrentAmount function, there is an unchecked block which skips underflow checks as startTime <= block.timestamp < endTime #114

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

In the _locateCurrentAmount function, there is an unchecked block which skips underflow checks as startTime <= block.timestamp < endTime, but if the condition is not upheld, the duration, elapsed, and remaining variables will underflow and can cause unexpected behavior.

Proof of Concept

function _locateCurrentAmount(

uint256 startAmount,

uint256 endAmount,

uint256 startTime,

uint256 endTime,

bool roundUp

) internal view returns (uint256 amount) {

// Only modify end amount if it doesn't already equal start amount.

if (startAmount != endAmount) {

    // Declare variables to derive.

    uint256 duration;

    uint256 elapsed;

    uint256 remaining;

    require(startTime <= block.timestamp && block.timestamp < endTime, "Invalid start and end time");

    // Derive the duration for the order and check for underflow

    if (endTime < startTime) {

        duration = 0;

    } else {

        duration = endTime - startTime;

    }

    // Derive time elapsed since the order started & check for underflow

    if (block.timestamp < startTime) {

        elapsed = 0;

    } else {

        elapsed = block.timestamp - startTime;

    }

    // Derive time remaining until

Tools Used

vs code

Recommended Mitigation Steps

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

dup #28

c4-judge commented 1 year ago

HickupHH3 marked the issue as unsatisfactory: Insufficient proof