code-423n4 / 2021-10-tempus-findings

0 stars 0 forks source link

Unnecessary checked arithmetic in for loops #41

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

pants

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.

RedFox20 commented 2 years ago

There is only one function where this is relevant, getSwapAmountToEndWithEqualShares() and it's compiled using Solidity 0.7.6, which does not have checked arithmetic, so this solution does not apply.

0xean commented 2 years ago

confirmed sponsors rebuttal. Closing.