Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

xiaoming90 - Rounding differences when computing the invariant #17

Open
github-actions bot opened this issue Jan 13, 2023 · 1 comment
Open

xiaoming90 - Rounding differences when computing the invariant #17

github-actions bot opened this issue Jan 13, 2023 · 1 comment
Labels
High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@github-actions
Copy link

xiaoming90

high

Rounding differences when computing the invariant

Summary

The invariant used within Boosted3Token vault to compute the spot price is not aligned with the Balancer's ComposableBoostedPool due to rounding differences. The spot price is used to verify if the pool has been manipulated before executing certain key vault actions (e.g. settle vault, reinvest rewards). In the worst-case scenario, it might potentially fail to detect the pool has been manipulated as the spot price computed might be inaccurate.

Vulnerability Detail

The Boosted3Token leverage vault relies on the old version of the StableMath._calculateInvariant that allows the caller to specify if the computation should round up or down via the roundUp parameter.

https://github.com/sherlock-audit/2022-12-notional/blob/main/contracts/vaults/balancer/internal/math/StableMath.sol#L28

File: StableMath.sol
28:     function _calculateInvariant(
29:         uint256 amplificationParameter,
30:         uint256[] memory balances,
31:         bool roundUp
32:     ) internal pure returns (uint256) {
33:         /**********************************************************************************************
34:         // invariant                                                                                 //
35:         // D = invariant                                                  D^(n+1)                    //
36:         // A = amplification coefficient      A  n^n S + D = A D n^n + -----------                   //
37:         // S = sum of balances                                             n^n P                     //
38:         // P = product of balances                                                                   //
39:         // n = number of tokens                                                                      //
40:         *********x************************************************************************************/
41: 
42:         unchecked {
43:             // We support rounding up or down.
44:             uint256 sum = 0;
45:             uint256 numTokens = balances.length;
46:             for (uint256 i = 0; i < numTokens; i++) {
47:                 sum = sum.add(balances[i]);
48:             }
49:             if (sum == 0) {
50:                 return 0;
51:             }
52: 
53:             uint256 prevInvariant = 0;
54:             uint256 invariant = sum;
55:             uint256 ampTimesTotal = amplificationParameter * numTokens;
56: 
57:             for (uint256 i = 0; i < 255; i++) {
58:                 uint256 P_D = balances[0] * numTokens;
59:                 for (uint256 j = 1; j < numTokens; j++) {
60:                     P_D = Math.div(Math.mul(Math.mul(P_D, balances[j]), numTokens), invariant, roundUp);
61:                 }
62:                 prevInvariant = invariant;
63:                 invariant = Math.div(
64:                     Math.mul(Math.mul(numTokens, invariant), invariant).add(
65:                         Math.div(Math.mul(Math.mul(ampTimesTotal, sum), P_D), _AMP_PRECISION, roundUp)
66:                     ),
67:                     Math.mul(numTokens + 1, invariant).add(
68:                         // No need to use checked arithmetic for the amp precision, the amp is guaranteed to be at least 1
69:                         Math.div(Math.mul(ampTimesTotal - _AMP_PRECISION, P_D), _AMP_PRECISION, !roundUp)
70:                     ),
71:                     roundUp
72:                 );
73: 
74:                 if (invariant > prevInvariant) {
75:                     if (invariant - prevInvariant <= 1) {
76:                         return invariant;
77:                     }
78:                 } else if (prevInvariant - invariant <= 1) {
79:                     return invariant;
80:                 }
81:             }
82:         }
83: 
84:         revert CalculationDidNotConverge();
85:     }

Within the Boosted3TokenPoolUtils._getSpotPrice and Boosted3TokenPoolUtils._getValidatedPoolData functions, the StableMath._calculateInvariant is computed rounding up.

https://github.com/sherlock-audit/2022-12-notional/blob/main/contracts/vaults/balancer/internal/math/Stable2TokenOracleMath.sol#L15

File: Boosted3TokenPoolUtils.sol
76:     function _getSpotPrice(
77:         ThreeTokenPoolContext memory poolContext, 
78:         BoostedOracleContext memory oracleContext,
79:         uint8 tokenIndex
80:     ) internal pure returns (uint256 spotPrice) {
..SNIP..
88:         uint256[] memory balances = _getScaledBalances(poolContext);
89:         uint256 invariant = StableMath._calculateInvariant(
90:             oracleContext.ampParam, balances, true // roundUp = true
91:         );

https://github.com/sherlock-audit/2022-12-notional/blob/main/contracts/vaults/balancer/internal/pool/Boosted3TokenPoolUtils.sol#L284

File: Boosted3TokenPoolUtils.sol
284:     function _getValidatedPoolData(
285:         ThreeTokenPoolContext memory poolContext,
286:         BoostedOracleContext memory oracleContext,
287:         StrategyContext memory strategyContext
288:     ) internal view returns (uint256 virtualSupply, uint256[] memory balances, uint256 invariant) {
289:         (virtualSupply, balances) =
290:             _getVirtualSupplyAndBalances(poolContext, oracleContext);
291: 
292:         // Get the current and new invariants. Since we need a bigger new invariant, we round the current one up.
293:         invariant = StableMath._calculateInvariant(
294:             oracleContext.ampParam, balances, true // roundUp = true
295:         );

However, Balancer has since migrated its Boosted3Token pool from the legacy BoostedPool structure to a new ComposableBoostedPool contract.

The new ComposableBoostedPool contract uses a newer version of the StableMath library where the StableMath._calculateInvariant function always rounds down.

https://etherscan.io/address/0xa13a9247ea42d743238089903570127dda72fe44#code#F16#L57

    function _calculateInvariant(uint256 amplificationParameter, uint256[] memory balances)
        internal
        pure
        returns (uint256)
    {
        /**********************************************************************************************
        // invariant                                                                                 //
        // D = invariant                                                  D^(n+1)                    //
        // A = amplification coefficient      A  n^n S + D = A D n^n + -----------                   //
        // S = sum of balances                                             n^n P                     //
        // P = product of balances                                                                   //
        // n = number of tokens                                                                      //
        **********************************************************************************************/

        // Always round down, to match Vyper's arithmetic (which always truncates).

        uint256 sum = 0; // S in the Curve version
        uint256 numTokens = balances.length;
        for (uint256 i = 0; i < numTokens; i++) {
            sum = sum.add(balances[i]);
        }
        if (sum == 0) {
            return 0;
        }
        ..SNIP..

Thus, Notional round up when calculating the invariant while Balancer's ComposableBoostedPool round down when calculating the invariant. This inconsistency will result in a different invariant

Impact

The invariant is used to compute the spot price to verify if the pool has been manipulated before executing certain key vault actions (e.g. settle vault, reinvest rewards). If the inputted invariant is inaccurate, the spot price computed might not be accurate and might not match the actual spot price of the Balancer Pool. In the worst-case scenario, it might potentially fail to detect the pool has been manipulated and the trade proceeds to execute against the manipulated pool leading to a loss of assets.

Code Snippet

https://github.com/sherlock-audit/2022-12-notional/blob/main/contracts/vaults/balancer/internal/math/StableMath.sol#L28

https://github.com/sherlock-audit/2022-12-notional/blob/main/contracts/vaults/balancer/internal/math/Stable2TokenOracleMath.sol#L15

https://github.com/sherlock-audit/2022-12-notional/blob/main/contracts/vaults/balancer/internal/pool/Boosted3TokenPoolUtils.sol#L284

Tool used

Manual Review

Recommendation

To avoid any discrepancy in the result, ensure that the StableMath library used by Balancer's ComposableBoostedPool and Notional's Boosted3Token leverage vault are aligned, and the implementation of the StableMath functions is the same between them.

@weitianjie2000
Copy link

valid, will fix

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

3 participants