Ocean Protocol v3

CertiK8 Oct 2020Files named in the report

CertiK reviewed Ocean Protocol (Ocean Protocol v3) on 8 Oct 2020: 60 findings, 0 critical; 30 marked fixed, 30 acknowledged.

No open critical findings

Open the report Open Ocean Protocol’s security record

Report hosted on skynet.certik.com.

Trustblock does not review code.

Findings60

Statuses are the security firm’s own, from its report.

Severity	Acknowledged	Fixed
Critical	0	0
High	1	0
Medium	0	0
Low	29	30

LowActivate / Deactivate to Toggle FunctionFixed

The linked functions contain identical statements with the core differentiator being the boolean literal they utilize.

LowAssembly Block DocumentationFixed

The purpose of the literals within the linked assembly block should be properly documented before proper evaluation of its functionality is conducted.

LowDuplicate Mapping LookupsFixed

All members of the Exchange struct are accessed and each mapping lookup operation as well as data retrieval costs significant gas.

LowEmpty ConstructorFixed

Empty constructors are redundant as they are directly implied even if not provided.

LowEmpty ConstructorFixed

Empty constructors are redundant as they are directly implied even if not provided.

LowERC20 Specification IncompatibilityFixed

The ERC20 specification denotes that the decimals of a token should be of type uint8 whereas here both the getter as well as the storage variable are declared as uint256 . This will cause complete incompatibility with the ERC20 specification as external contracts will not be able to properly retrieve the decimals of the contract.

LowInconsistent Error HandlingFixed

Throughout the contracts of the project proper sanitization of input variables during construction is conducted whereby the addresses of contracts are ensured to be different than zero. However, the error message provided is of a completely different convention than DTFactory.sol for instance.

LowInefficient Greater-Than Comparisons w/ ZeroFixed

The linked code statements conduct a greater-than comparison between zero and an unsigned integer.

LowInefficient Greater-Than Comparisons w/ ZeroFixed

The linked code statements conduct a greater-than comparison between zero and an unsigned integer.

LowInefficient Greater-Than Comparisons w/ ZeroFixed

The linked code statements conduct a greater-than comparison between zero and an unsigned integer.

Show 10 more

LowInexistent IndexingFixed

The linked event declarations contain no indexed variables.

LowLiteral AssignmentFixed

The _decimals variable is being assigned to the literal 18.

LowMisleading Event Variable NamesFixed

The event declarations state that a timestamp type variable is emitted along the event whereas a block.number is provided instead of a block.timestamp wherever the events are emitted.

LowPause / Unpause to Toggle FunctionFixed

The linked functions contain identical statements with the core differentiator being the boolean literal they utilize.

LowPull-over-Push PatternFixed

Directly replacing the minter address of a contract is dangerous as a single misinputed character can freeze the minting functions of the contract indefinitely.

LowRedundant Event VariableFixed

The linked `TokenRegistered` event includes the `block.number` within the variables it emits.

LowRedundant Event VariableFixed

The linked BPoolRegistered event includes the block.number within the variables it emits indexed as well.

LowRedundant Utilization of abi.encodePackedFixed

The abi.encodePacked function solely makes sense when the variables it operates on can be tightly packed.

LowStruct OptimizationFixed

The Exchange struct can be further optimized.

LowUnlocked Compiler VersionFixed

The compiler version utilized in this file uses the “^” prefix specifier, denoting that a compiler at or above the version included after the specifier should be used to compile the contracts. It is a general practise to instead lock the compiler at a specific version rather than allow a range of compiler versions to be utilized to avoid compiler-specific bugs and be able to identify ones more easily.

Show 10 more

LowUnlocked Compiler VersionFixed

The compiler version utilized in this file uses the “^” prefix specifier, denoting that a compiler at or above the version included after the specifier should be used to compile the contracts. It is a general practice to instead lock the compiler at a specific version rather than allow a range of compiler versions to be utilized to avoid compiler-specific bugs and be able to identify ones more easily.

LowUnlocked Compiler VersionFixed

TThe compiler version utilized in this file uses the “^” prefix specifier, denoting that a compiler at or above the version included after the specifier should be used to compile the contracts. It is a general practise to instead lock the compiler at a specific version rather than allow a range of compiler versions to be utilized to avoid compiler-specific bugs and be able to identify ones more easily.

LowUnsanitized VariablesFixed

The input variables of the startOrder function are unsanitized, meaning that anyone is capable of setting a zero feePercentage as well as a custom feeCollector address.

LowUser Defined GettersFixed

In general, it is a good practise to avoid user-defined getters and instead rely on compilergenerated ones via the public keyword as they are far more maintainable and aid in the legibility of the codebase

LowVariable OrderingFixed

Within Solidity, the order of variable declaration is important within a contract as it dictates how the compiler will tightly-pack the variables it is provided with. While the layout of the contract does not appear to follow a particular convention, it has tightly packed most slots possible except one.

LowVisibility Specifiers MissingFixed

The visibility specifiers for the linked variables should be explicitly set as it is standard security practise and aids in the readability of the codebase.