search

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

1 stars 0 forks source link

QA Report #186

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

[QA-01] Consider securing your JavaScript supply chain

Description

JavaScript package supply chain attack is still a threat.

Recommended Mitigation Steps

Consider using Socket (https://socket.dev/)

-----------------------------------------------------------------

[QA-02] Improve readability with constants

Description

The following variables can be made constant and be populated in compile time. https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/ConsiderationBase.sol#L23-L30

There is no need having a function and runtime computation at deploy time.

It also saves gas.

Recommended Mitigation Steps

Consider computing hashes at compile time.

-----------------------------------------------------------------

[QA-03] Improve readability in inequality check

Description

https://github.com/code-423n4/2022-05-opensea-seaport/blob/main/contracts/lib/Verifiers.sol#L43

This is just my opinion on readability: Time inequality should be in increasing direction.

Recommended Mitigation Steps

Consider rewriting:

if (startTime > block.timestamp || endTime <= block.timestamp) {
    ...
}

to this:

if (block.timestamp < startTime || endTime <= block.timestamp) {
    ...
}

-----------------------------------------------------------------

GalloDaSballo commented 1 year ago

Disagree with he findings: 1) No POC, no explanation 2) Immutable is a constant defined at constructor time, disagree with the "readability" take 3) Disagree that the refactoring helps

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.