DeFiHardhatFoundry
250,000 USDC
View results
Submission Details
Severity: low
Invalid

Integer Division Rounding Error in Unripe Token Conversion Leads to Potential Value Loss and Inconsistent BDV Calculations

Summary

The LibUnripe library contains a function unripeToUnderlying that converts unripe tokens to their underlying value. This function is used in the BDVFacet contract to calculate the Bean Denominated Value (BDV) of unripe LP tokens and unripe Beans. Due to Solidity's integer division, this conversion introduces a rounding error that can lead to inconsistent BDV calculations and potential value loss for users.

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/8c8710df547f7d7c5dd82c5381eb6b34532e4484/protocol/contracts/libraries/LibUnripe.sol#L85C3-L94C6

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/8c8710df547f7d7c5dd82c5381eb6b34532e4484/protocol/contracts/beanstalk/silo/BDVFacet.sol#L30C3-L38C6

In LibUnripe.sol

function unripeToUnderlying(
address unripeToken,
uint256 unripe,
uint256 supply
) internal view returns (uint256 underlying) {
AppStorage storage s = LibAppStorage.diamondStorage();
underlying = s.sys.silo.unripeSettings[unripeToken].balanceOfUnderlying.mul(unripe).div(
supply
);
}

In BDVFacet.sol

function unripeLPToBDV(uint256 amount) public view returns (uint256) {
amount = LibUnripe.unripeToUnderlying(
C.UNRIPE_LP,
amount,
IBean(C.UNRIPE_LP).totalSupply()
);
amount = LibWellBdv.bdv(LibBarnRaise.getBarnRaiseWell(), amount);
return amount;
}

Proof of Concept

Consider the following scenario:

  • balanceOfUnderlying: 1000

  • unripe: 3

  • supply: 7

Calculation:

  1. 1000 * 3 = 3000

  2. 3000 / 7 = 428 (rounded down from 428.571428...)

The function returns 428 underlying tokens instead of the mathematically correct 428.571428..., resulting in a loss of 0.571428... tokens (approximately 0.13% of the correct value).

Coded PoC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
import "forge-std/Test.sol";
import "../contracts/libraries/LibUnripe.sol";
import "../contracts/BDVFacet.sol";
contract UnripeConversionTest is Test {
LibUnripe libUnripe;
BDVFacet bdvFacet;
function setUp() public {
libUnripe = new LibUnripe();
bdvFacet = new BDVFacet();
}
function testUnripeToUnderlyingRoundingError() public {
uint256 balanceOfUnderlying = 1000;
uint256 unripe = 3;
uint256 supply = 7;
uint256 result = libUnripe.unripeToUnderlying(address(0), unripe, supply);
uint256 expectedExact = (balanceOfUnderlying * unripe * 1e18) / supply;
uint256 actualScaled = result * 1e18;
assertLt(actualScaled, expectedExact, "Result should be less than mathematically exact value");
assertGt(expectedExact - actualScaled, 0.1e18, "Error should be greater than 0.1");
assertLt(expectedExact - actualScaled, 0.6e18, "Error should be less than 0.6");
}
function testBDVCalculationDiscrepancy() public {
uint256 amount = 1000000; // 1 million unripe LP tokens
uint256 bdv1 = bdvFacet.unripeLPToBDV(amount);
uint256 bdv2 = bdvFacet.unripeLPToBDV(amount / 2) * 2;
assertNotEq(bdv1, bdv2, "BDV calculations should not be exactly equal due to rounding");
assertLt(absDifference(bdv1, bdv2), amount / 1000, "Difference should be less than 0.1%");
}
function absDifference(uint256 a, uint256 b) internal pure returns (uint256) {
return a > b ? a - b : b - a;
}
}

These tests demonstrate the rounding error in unripeToUnderlying and its impact on BDV calculations. The first test shows that the conversion result is less than the mathematically exact value, with an error between 0.1 and 0.6 tokens per 1000 underlying. The second test reveals that calculating BDV for a large amount all at once versus in two halves can lead to discrepancies, albeit small ones.

Impact

1: The rounding error can lead to slightly different BDV values for the same amount of tokens in different transactions.

2: Users may receive slightly less value than expected when converting unripe tokens.

Tools Used

Manual Review

Recommendations

1: Consider using a fixed-point arithmetic library like PRBMath for more precise calculations.

2: Alternatively, increase precision by scaling up values before division and scaling down afterward:

function unripeToUnderlying(
address unripeToken,
uint256 unripe,
uint256 supply
) internal view returns (uint256 underlying) {
AppStorage storage s = LibAppStorage.diamondStorage();
underlying = s.sys.silo.unripeSettings[unripeToken].balanceOfUnderlying.mul(unripe).mul(1e18).div(supply);
underlying = underlying.div(1e18);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational/Gas

Invalid as per docs https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.