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

1 stars 0 forks source link

Unsafe uint128 casting may overflow #112

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

sirhashalot

Vulnerability details

Impact

The _calcRewardIntegral function casts intermediate reward values from uint256 to uint128 and vice versa several times. Because OpenZeppelin SafeCast is not used, casting from uint256 to uint128 may overflow if a large reward value is being calculate. This overflow could result in users receiving less rewards than they are owed.

Proof of Concept

There are 4 uint128 casting operations and 2 uint256 casting operations in the _calcRewardIntegral function of ConvexStakingWrapper.sol.

Recommended Mitigation Steps

Because reward values are an important part of this protocol, use the OpenZeppelin SafeCast library to prevent unexpected overflows when casting. SafeMath and Solidity 0.8.* handles overflows for basic math operations but not for casting.

alcueca commented 2 years ago

A better solution would be to store values in uint128 (if that is required), then cast to uint256 for calculations to gain range, and then cast with safety guards back to uint128 at the end.

GalloDaSballo commented 2 years ago

I agree with the finding, personally I'd recommend the sponsor to use the higher uint256 type unless they can reliably use uint128 without castings.

That said, the warden identified castings that "could" be dangerous and in lack of any specific exploit I believe low severity to be appropriate