Summary
Division before multiplication can cause precision loss.
Vulnerability Details
In the LibUnripe.sol library, getPenalizedUnderlying() function is implemented to calculate the the penalized amount of Ripe Tokens.
./LibUnripe.sol
function getPenalizedUnderlying(
address unripeToken,
uint256 amount,
uint256 supply
) internal view returns (uint256 redeem) {
require(isUnripe(unripeToken), "not vesting");
AppStorage storage s = LibAppStorage.diamondStorage();
uint256 totalUsdNeeded = unripeToken == C.UNRIPE_LP ? LibFertilizer.getTotalRecapDollarsNeeded(supply)
: LibFertilizer.getTotalRecapDollarsNeeded();
uint256 underlyingAmount = s.u[unripeToken].balanceOfUnderlying;
redeem = underlyingAmount.mul(s.recapitalized).div(totalUsdNeeded).mul(amount).div(supply);
if(redeem > underlyingAmount) redeem = underlyingAmount;
}
Here, redeem is calcuated as:
redeem = underlyingAmount.mul(s.recapitalized).div(totalUsdNeeded).mul(amount).div(supply);
It divides totalUsdNeeded first and then multiplies amount and again divides supply. Thus, not following the optimal solidity design pattern as solidity's integer division truncates. Thus, performing division before multiplication can lead to precision loss.
Impact
It can cause loss of precision and result in redeem amount being calculated to be less than it should. As, getPenalizedUnderlying() function is a very crucial function and miscalculation of redeem function due to precision loss can cause a lot of miscalculations across the entire protocol. Thus, the medium severity.
Tools Used
Manual Analysis
Recommendations
Modify the getPenalizedUnderlying() function such as:
./LibUnripe.sol
function getPenalizedUnderlying(
address unripeToken,
uint256 amount,
uint256 supply
) internal view returns (uint256 redeem) {
require(isUnripe(unripeToken), "not vesting");
AppStorage storage s = LibAppStorage.diamondStorage();
// getTotalRecapDollarsNeeded() queries for the total urLP supply which is burned upon a chop
// If the token being chopped is unripeLP, getting the current supply here is inaccurate due to the burn
// Instead, we use the supply passed in as an argument to getTotalRecapDollarsNeeded since the supply variable
// here is the total urToken supply queried before burnning the unripe token
uint256 totalUsdNeeded = unripeToken == C.UNRIPE_LP ? LibFertilizer.getTotalRecapDollarsNeeded(supply)
: LibFertilizer.getTotalRecapDollarsNeeded();
// chop rate = total redeemable * (%DollarRecapitalized)^2 * share of unripe tokens
// redeem = totalRipeUnderlying * (usdValueRaised/totalUsdNeeded)^2 * UnripeAmountIn/UnripeSupply;
// But totalRipeUnderlying = CurrentUnderlying * totalUsdNeeded/usdValueRaised to get the total underlying
// redeem = currentRipeUnderlying * (usdValueRaised/totalUsdNeeded) * UnripeAmountIn/UnripeSupply
uint256 underlyingAmount = s.u[unripeToken].balanceOfUnderlying;
- redeem = underlyingAmount.mul(s.recapitalized).div(totalUsdNeeded).mul(amount).div(supply);
+ redeem = underlyingAmount.mul(s.recapitalized).mul(amount).div(totalUsdNeeded).div(supply);
// cap `redeem to `balanceOfUnderlying in the case that `s.recapitalized` exceeds `totalUsdNeeded`.
// this can occur due to unripe LP chops.
if(redeem > underlyingAmount) redeem = underlyingAmount;
}
