search

code-423n4 / 2022-05-opensea-seaport-findings

1 stars 0 forks source link

QA Report #165

Closed code423n4 closed 1 year ago

code423n4 commented 2 years ago

Low

[L-01]

Impact

Using the latest versions of solidity might make contracts susceptible to undiscovered compiler bugs. All contracts in scope are currently using solidity version 0.8.13.

Recommendation

Consider using solidity version 0.8.4 - 0.8.7 to avoid unexpected bugs.

[L-02] Missing checks for address(0) when transfering tokens/eth.

Impact

Address type parameters should include a zero-address check otherwise contract functionality may become inaccessible or tokens burnt forever.

Proof of Concept

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/Executor.sol#L222-L224

function _transferEth(address payable to, uint256 amount) internal {
        // Ensure that the supplied amount is non-zero.
        _assertNonZeroAmount(amount);

Recommendation

Add address(0) check.

Non-Critical

[N-01] Missing Natspec

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/GettersAndDerivers.sol#L281-L287

/**
     * @dev Internal view function to get the EIP-712 domain separator. If the
     *      chainId matches the chainId set on deployment, the cached domain
     *      separator will be returned; otherwise, it will be derived from
     *      scratch.
     */
    function _domainSeparator() internal view returns (bytes32) {

Missing @return tag.

Tools Used

Manual.

GalloDaSballo commented 1 year ago

Disagree with all points 0.8.13 may be one of the most stable solidity releases

address(0) are never used in the codebase and the sponsor is a proponent of that

Natspec is always optional

HardlyDifficult commented 1 year ago

The bar for QA reports in this contest is at least 2 valid non-critical findings or at least one valid low risk finding. Per the comments above, this submission is below that bar -- closing as invalid.