Nested Finance contest - samruna's results

The one-stop Defi app to build, manage and monetize your portfolio.

General Information

Platform: Code4rena

Start Date: 10/02/2022

Pot Size: $30,000 USDC

Total HM: 5

Participants: 24

Period: 3 days

Judge: harleythedog

Total Solo HM: 3

Id: 86

League: ETH

Nested Finance

Findings Distribution

Researcher Performance

Rank: 16/24

Findings: 1

Award: $153.03

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: pauliax

Also found by: 0xliumin, Dravee, GreyArt, IllIllI, Omik, ShippooorDAO, WatchPug, bobi, csanuragjain, gzeon, kenzo, rfa, robee, samruna

Labels

bug
QA (Quality Assurance)
sponsor confirmed

Awards

153.0348 USDC - $153.03

External Links

Gas fee improvement https://github.com/code-423n4/2022-02-nested/blob/main/contracts/NestedFactory.sol#L100 there is no need to use extra variable operatorsCache to check if operator exists.

https://github.com/code-423n4/2022-02-nested/blob/main/contracts/NestedFactory.sol#L111 In the removeOperator function, no need for this line uint256 operatorsLength = operators.length; Use operators.length directly in the loop

https://github.com/code-423n4/2022-02-nested/blob/main/contracts/NestedFactory.sol#L177 getAssetTokensLength() function is not called from the contract. If not needed it can removed or should be made external

https://github.com/code-423n4/2022-02-nested/blob/main/contracts/NestedFactory.sol#L191 Function tokenHoldings() is not called from within the contract. If not needed it can be removed or should be made external

https://github.com/code-423n4/2022-02-nested/blob/main/contracts/NestedAsset.sol#L52 Function originalOwner() is not called from within the contract. If not needed it can be removed or should be made external

https://github.com/code-423n4/2022-02-nested/blob/main/contracts/OperatorResolver.sol#L16 Add empty string check for this function

https://github.com/code-423n4/2022-02-nested/blob/main/contracts/OperatorResolver.sol#L57 If bith names.length and operatorsToImport.length == 0, this check passes. Consider adding empty check to both structures

https://github.com/code-423n4/2022-02-nested/blob/main/contracts/FeeSplitter.sol#L204 Nested _addShare _sendFees calls _addShare But in sendFeesWithRoyalties(), _addShare is called again after _sendFees. This can lead to nested executed. Please consider refactoring this code.

#0 - maximebrugel

2022-02-15T18:22:19Z

"there is no need to use extra variable operatorsCache to check if operator exists." (Disputed)

Cheaper to read in local variable.

"Use operators.length directly in the loop" (Disputed)

Cheaper to read in local variable.

"getAssetTokensLength() function is not called from the contract (...)" (Confirmed)

"Function tokenHoldings() is not called from within the contract (...)" (Confirmed)

"Function originalOwner() is not called from within the contract" (Confirmed)

"Add empty string check for this function" (Disputed)

This is view, not needed.

"(...) Consider adding empty check to both structures" (Disputed)

Yes, but not an issue in this case.

"in sendFeesWithRoyalties(), _addShare is called again after _sendFees. (...)" (Disputed)

I feel it's better that way.

#1 - harleythedogC4

2022-03-01T02:09:54Z

My personal judgements:

  1. "there is no need to use extra variable operatorsCache to check if operator exists" - agree with sponsor, this suggestion would increase gas usage and doesn't really help at all. Invalid.
  2. "Use operators.length directly in the loop". Same as 1. Invalid.
  3. "getAssetTokensLength() function is not called from the contract". Valid and non-critical.
  4. "Function tokenHoldings() is not called from within the contract". Valid and non-critical.
  5. "Function originalOwner() is not called from within the contract". Valid and non-critical.
  6. "Add empty string check for this function". Not enough explanation to see why this helps in any way. Invalid.
  7. Consider adding empty check to both structures. Perhaps the admin calls the function with zero length input by accident and does not notice, function will still pass. Valid and very-low-critical.
  8. "Nested _addShare". Not enough explanation to see why this helps in any way. Invalid.

#2 - harleythedogC4

2022-03-03T01:53:58Z

Adding in the reduced severity #23: 9. "updateShareHolder logic does not seem correct". Just code consistency. Valid and non-critical.

#3 - harleythedogC4

2022-03-03T02:24:46Z

Now, here is the methodology I used for calculating a score for each QA report. I first assigned each submission to be either non-critical (1 point), very-low-critical (5 points) or low-critical (10 points), depending on how severe/useful the issue is. The score of a QA report is the sum of these points, divided by the maximum number of points achieved by a QA report. This maximum number was 26 points, achieved by https://github.com/code-423n4/2022-02-nested-findings/issues/66.

The number of points achieved by this report is 9 points. Thus the final score of this QA report is (9/26)*100 = 35.

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter