code-423n4 / 2021-11-badgerzaps-findings

0 stars 0 forks source link

Unnecessary checked arithmetic in for loops #66

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

pants

Vulnerability details

Vulnerability details

There is no risk of overflow caused by increamenting the iteration index in for loops (the i++ in for (uint256 i = 0; i < numIterations; i++)).

Impact

Increaments perform overflow checks that are not necessary in this case.

Tool Used

Manual code review.

Recommended Mitigation Steps

Surround the increament expressions with an unchecked { ... } block to avoid the default overflow checks. For example, change the loop

for (uint256 i = 0; i < numIterations; i++) {
    // ...
}

to

for (uint256 i = 0; i < numIterations;) {
    // ...
    unchecked { ++i; }
}

It is a little less readable but it saves a significant amount of gas.

Zap.sol line 70 for example

GalloDaSballo commented 2 years ago

Disagree with the finding, the warden thinks we're using 0.8, we're using 0.6.12 which means we don't have automatic overflow checks

0xleastwood commented 2 years ago

Agree with sponsor, unchecked statements are not supported in Solidity version 0.6.12.