search

code-423n4 / 2022-06-nested-findings

0 stars 1 forks source link

QA Report #60

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

1 Unused parameter

Impact efficiency code and reduce gas cost

Proof of Concept https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L466

Tool Used Remix

Recommended Mitigation Steps remove unnecessary code. i suggest to change

     (bool success, uint256[] memory amounts) = callOperator(_order, _inputToken, _outputToken);

to

    (bool success, ) = callOperator(_order, _inputToken, _outputToken);

2 Missing param comment

impact transferOwnership function have natspec comment which is missing the newOwner function parameter. Issues with comments are low risk based on Code4rena risk categories.

Proof of Concept https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/abstracts/OwnableProxyDelegation.sol#L54-L59

Tool Manual Review

Recommended Mitigation Steps Add natspec comments include newOwner parameter.

3 Missing param operatorStorage in constructor

Impact the operatorStorage can't be initialize by constructor. and made the logic doesn't work properly.

Proof of Concept https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/operators/Yearn/YearnCurveVaultOperator.sol#L31-L36

Tool Manual review

Recommended Mitigation Steps because in the constructor parameter mention operatorStorage to initialize. so i suggest to add param operatorStorage on constructor.

4 Missing state operatorResolver

Impact the constructor initialize operatorResolver but the state operatorResolver was missing, and caused of it made opeartion doesn't work properly.

Proof of Concept https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L57-L75

Tool Manual review

Recommended Mitigation Steps i suggest to add state about operatorResolver, so that the constructor can work to initialize param operatorResolver.

5 feeSplitter must be immutable

Impact the feeSplitter can't be initialize by constructor.

Proof of Concept https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L29

Tool Manual review

Recommended Mitigation Steps the state must add immutable because in the constructor parameter mention feeSplitter to initialize. so i suggest to add immutable on it.

obatirou commented 2 years ago

1.Unused parameter (disputed)

the parameter amounts is used 2 lines after We cannot remove it

obatirou commented 2 years ago

3 Missing param operatorStorage in constructor (disputed)

The statement is not clear and the logic works Why can’t it be initialized by constructor ?

obatirou commented 2 years ago

4 Missing state operatorResolver (disputed)

The resolver is part of the MixinOperatorResolver

obatirou commented 2 years ago

5 feeSplitter must be immutable (disputed)

We can modify the address of the fee splitter Hence cannot be immutable

obatirou commented 2 years ago

Missing param comment (duplicate)

https://github.com/code-423n4/2022-06-nested-findings/issues/84#issuecomment-1165712399