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 ConvexStakingWrapper.sol:_calcRewardIntegral(), there's this if/else statement:

File: ConvexStakingWrapper.sol
233:             if (_isClaim || userI < rewardIntegral) {
234:                 if (_isClaim) {
235:                     uint256 receiveable = reward.claimable_reward[_accounts[u]] +
236:                         ((_balances[u] * (uint256(rewardIntegral) - userI)) / 1e20);
237:                     if (receiveable > 0) {
238:                         reward.claimable_reward[_accounts[u]] = 0;
239:                         IERC20(reward.reward_token).safeTransfer(_accounts[u], receiveable);
240:                         bal = bal - receiveable;
241:                     }
242:                 } else {
243:                     reward.claimable_reward[_accounts[u]] =
244:                         reward.claimable_reward[_accounts[u]] +
245:                         ((_balances[u] * (uint256(rewardIntegral) - userI)) / 1e20); //@audit-info can't underflow as the only way to get here is if userI < rewardIntegral
246:                 }
247:                 reward.reward_integral_for[_accounts[u]] = rewardIntegral;
248:             }

As you can see on line 245 (@audit-info), there's a substraction that can't underflow, as the only way to get to this line is if userI < rewardIntegral. This substraction should get computed inside an unchecked block and stored in a variable, which would then be used in the checked calculation.

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

This is very similar to #62 and #65

alcueca commented 2 years ago

Very similar, but up to the judge to group them

GalloDaSballo commented 2 years ago

Leaving https://github.com/code-423n4/2022-01-yield-findings/issues/67 valid but marking the rest as invalid as they are duplicates by the same warden

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