code-423n4 / 2021-12-defiprotocol-findings

0 stars 0 forks source link

Unnecessary checked arithmetic in for loops #121

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

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

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

Recommendation

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

https://github.com/code-423n4/2021-12-defiprotocol/blob/205d3766044171e325df6a8bf2e79b37856eece1/contracts/contracts/Basket.sol#L289-L293

for (uint256 i = 0; i < weights.length; i++) {
    uint256 tokenAmount = amount * weights[i] * ibRatio / BASE / BASE;
    require(tokenAmount > 0);
    IERC20(tokens[i]).safeTransferFrom(from, address(this), tokenAmount);
}

to

for (uint256 i = 0; i < weights.length;) {
    uint256 tokenAmount = amount * weights[i] * ibRatio / BASE / BASE;
    require(tokenAmount > 0);
    IERC20(tokens[i]).safeTransferFrom(from, address(this), tokenAmount);
    unchecked { ++i; }
}