search

code-423n4 / 2022-05-sturdy-findings

7 stars 3 forks source link

QA Report #2

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Impact

[1] By default, function types and state variables/constants are internal, so the internal keyword can be omitted.

Affected code:

  1. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/CollateralAdapter.sol#L24
  2. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/CollateralAdapter.sol#L27
  3. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/CollateralAdapter.sol#L29
  4. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/CollateralAdapter.sol#L49
  5. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/ConvexCurveLPVault.sol#L28
  6. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/ConvexCurveLPVault.sol#L29
  7. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/ConvexCurveLPVault.sol#L30
  8. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/ConvexCurveLPVault.sol#L48
  9. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/ConvexCurveLPVault.sol#L55
  10. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/ConvexCurveLPVault.sol#L117
  11. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/ConvexCurveLPVault.sol#L145
  12. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/ConvexCurveLPVault.sol#L146
  13. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/ConvexCurveLPVault.sol#L148
  14. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/ConvexCurveLPVault.sol#L161
  15. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/ConvexCurveLPVault.sol#L173
  16. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/GeneralVault.sol#L49
  17. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/GeneralVault.sol#L52
  18. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/GeneralVault.sol#L53
  19. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/YieldManager.sol#L36
  20. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/YieldManager.sol#L37
  21. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/YieldManager.sol#L39
  22. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/YieldManager.sol#L46

Proof of Concept

Tools Used

Recommended Mitigation Steps

Impact

[2] Magic number, consider using named constant instead.

Affected code:

  1. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/GeneralVault.sol#L123
  2. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/LidoVault.sol#L48
  3. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/LidoVault.sol#L136

Proof of Concept

Tools Used

Recommended Mitigation Steps

Impact

[3] Consider using "_" separate digit capacity i.e "100000" could be replaced to "100_000". This increases code readability.

Affected code:

  1. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/YieldManager.sol#L48

Proof of Concept

Tools Used

Recommended Mitigation Steps

Impact

[4] Consider using IERC20 type instead of address. Or IERC20[] type instead of address[].

Affected code:

  1. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/ConvexCurveLPVault.sol#L28
  2. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/ConvexCurveLPVault.sol#L29
  3. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/ConvexCurveLPVault.sol#L37
  4. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/ConvexCurveLPVault.sol#L93
  5. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/ConvexCurveLPVault.sol#L94
  6. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/ConvexCurveLPVault.sol#L108
  7. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/YieldManager.sol#L43
  8. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/YieldManager.sol#L64
  9. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/YieldManager.sol#L93
  10. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/YieldManager.sol#L94
  11. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/YieldManager.sol#L106
  12. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/YieldManager.sol#L106
  13. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/YieldManager.sol#L195
  14. https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/YieldManager.sol#L202

Proof of Concept

Tools Used

Recommended Mitigation Steps

Impact

[5] Typo: variable name supposed to be 'decimals'.

Affected code:

  1. https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/GeneralVault.sol#L122

Proof of Concept

Tools Used

Recommended Mitigation Steps

Impact

[6] Consider reducing if nesting by having early continue/return and else contents clause can be placed right after. This increases readability of the code.

Affected code:

  1. https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/GeneralVault.sol#L220-L229
  2. https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/YieldManager.sol#L158-L167
  3. https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/LidoVault.sol#L128-L147

Proof of Concept

Tools Used

Recommended Mitigation Steps

Impact

[7] Usually when you leave function empty it is a good practice to place a comment inside brackets { /* reason why here is no code */ } Consider adding explanation in comments.

Affected code:

  1. https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/GeneralVault.sol#L246
  2. https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/GeneralVault.sol#L255
  3. https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/GeneralVault.sol#L265
  4. https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/LidoVault.sol#L24

Proof of Concept

Tools Used

Recommended Mitigation Steps

Impact

[8] Consider adding here require(msg.value == 0); since it is non-ETH token.

Affected code:

  1. https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/LidoVault.sol#L96

Proof of Concept

Tools Used

Recommended Mitigation Steps

Impact

[9] Concern: Isn't it better to break the for-loop instead of reverting whole transaction?

Affected code:

  1. https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/YieldManager.sol#L122

Proof of Concept

Tools Used

Recommended Mitigation Steps

Impact

[10] Brackets aren't necessary here, consider making this code one-liner.

Affected code:

  1. https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/YieldManager.sol#L199-L201

Proof of Concept

Tools Used

Recommended Mitigation Steps

HickupHH3 commented 2 years ago

NC issues: 1, 2, 3, 4, 5, 6, 7, 10 Low issues: #3, #4, #5, 9

8 has been bumped to medium severity