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

0 stars 0 forks source link

Unnecessary checked arithmetic in for loops #204

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 < _unbondingLockIds.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/livepeer/arbitrum-lpt-bridge/blob/ebf68d11879c2798c5ec0735411b08d0bea4f287/contracts/L1/gateway/L1Migrator.sol#L472-L479

for (uint256 i = 0; i < _unbondingLockIds.length; i++) {
    (uint256 amount, ) = bondingManager.getDelegatorUnbondingLock(
        _l1Addr,
        _unbondingLockIds[i]
    );

    total += amount;
}

to:

for (uint256 i = 0; i < _unbondingLockIds.length;) {
    (uint256 amount, ) = bondingManager.getDelegatorUnbondingLock(
        _l1Addr,
        _unbondingLockIds[i]
    );

    total += amount;

    unchecked { ++i; }
}
yondonfu commented 2 years ago

Likely won't make this change as we think it hurts readability.