code-423n4 / 2022-01-yield-findings

1 stars 0 forks source link

Gas in `CvxMining.sol:ConvertCrvToCvx()`: `reduction` can't underflow #66

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

Dravee

Vulnerability details

Impact

Increased gas cost due to unnecessary automatic underflow checks.

Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers.

When an overflow or an underflow isn't possible (as an example, when a comparison is made before the arithmetic operation, or the operation doesn't depend on user input), some gas can be saved by using an unchecked block.

https://docs.soliditylang.org/en/v0.8.10/control-structures.html#checked-or-unchecked-arithmetic

Proof of Concept

In CvxMining.sol:ConvertCrvToCvx(): the conditional statement at line 20 prevents an underflow from ever happening at line 22:

File: CvxMining.sol
20:         if (cliff < totalCliffs) {
21:             //for reduction% take inverse of current cliff
22:             uint256 reduction = totalCliffs - cliff; //@audit-info can't underflow

Therefore, line 22 should be inside an unchecked block.

Tools Used

VS Code

Recommended Mitigation Steps

Uncheck arithmetic operations when the risk of underflow or overflow is already contained by wrapping them in an unchecked block

devtooligan commented 2 years ago

similar to #62 #64 #65

alcueca commented 2 years ago

Disputed as stated in the readme.

GalloDaSballo commented 2 years ago

Leaving #67 valid but marking the rest as invalid as they are duplicates by the same warden