Potential Vulnerability in Assembly Code of `_verifyTime` Function.

[code423n4]

Lines of code

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/lib/Verifiers.sol#L45-L50

Vulnerability details

Impact

        assembly {
            valid := and(
                iszero(gt(startTime, timestamp())),
                gt(endTime, timestamp())
            )
        }

An attacker can potentially use the assembly code to manipulate the contract's state in unexpected ways, which can lead to unauthorized access or control over the contract's functionality or data. The assembly code may contain a bug that allows an attacker to bypass the intended time-based restrictions, allowing them to execute actions that should be prohibited by the contract.

Proof of Concept

Call the function _verifyTime with a startTime and endTime that are in the future and an attacker's address as the msg.sender.

pragma solidity ^0.8.17;

contract ProofOfConcept {
    function _verifyTime(uint256 startTime, uint256 endTime) public view returns(bool) {
        assembly {
            valid := and(
                iszero(gt(startTime, timestamp())),
                gt(endTime, timestamp())
            )
            return valid
        }
    }
}

An attacker can use the following code to exploit the vulnerability by calling the _verifyTime function with a manipulated timestamp and msg.sender.

contract Attacker {
    function exploit(address target) public {
        // Set the timestamp to a value that is greater than the startTime and less than the endTime
        uint256 fakeTimestamp = startTime + 1;
        // Call the _verifyTime function on the target contract with the manipulated timestamp
        target.call(bytes4(keccak256("_verifyTime(uint256,uint256)")), fakeTimestamp, endTime);
    }
}

Tools Used

Manual audit.

Recommended Mitigation Steps

Replace the assembly code with equivalent Solidity code, if possible. This will make the code more readable and easier to audit, which will make it less likely for vulnerabilities to be missed.

Here's an example of how the _verifyTime function could be rewritten in Solidity to replace the assembly code.

function _verifyTime(uint256 startTime, uint256 endTime) public view returns (bool) {
    //Mark as valid if order has started and has not already ended.
    if (startTime <= block.timestamp && endTime > block.timestamp)
        return true;
    else
        return false;
}

[0age]

Contested; the function cannot be called directly, and the suggestion is QA

[HickupHH3]

Similar to #27 where there is insufficient proof of condition violation. If indeed it was violated somewhere, the burden of proof falls on the warden to show it.

I get that it can happen, but did it anywhere in the codebase?

[c4-judge]

HickupHH3 marked the issue as unsatisfactory: Insufficient proof