sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

xiaoming90 - Boosted Balancer Leverage Vault Vulnerable To Price Manipulation #69

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

xiaoming90

high

Boosted Balancer Leverage Vault Vulnerable To Price Manipulation

Summary

The Boosted Balancer leverage vaults are vulnerable to price manipulation as one of the variables used to compute the price can be manipulated within a single block/transaction.

Vulnerability Detail

The design of the strategy vault must be resistant to price manipulation and flash-loan attacks.

It was observed that the token balance returned by the Boosted3TokenPoolUtils._getTimeWeightedPrimaryBalance function is not time-weighted even though the comments claim that it will return the time-weighted primary token balance for a given bptAmount and will use Chainlink (See Line 185-186). Thus, the Boosted3TokenPoolUtils._getTimeWeightedPrimaryBalance function is not flash-loan resistance.

The primaryAmount returned by Boosted3TokenPoolUtils._getTimeWeightedPrimaryBalance function is computed based on a number of variables. One of the variables used during the calculation is the virtualSupply.

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

File: Boosted3TokenPoolUtils.sol
185:     /// @notice Gets the time-weighted primary token balance for a given bptAmount
186:     /// @dev Boosted pool can't use the Balancer oracle, using Chainlink instead
187:     /// @param poolContext pool context variables
188:     /// @param oracleContext oracle context variables
189:     /// @param bptAmount amount of balancer pool lp tokens
190:     /// @return primaryAmount primary token balance
191:     function _getTimeWeightedPrimaryBalance(
192:         ThreeTokenPoolContext memory poolContext,
193:         BoostedOracleContext memory oracleContext,
194:         StrategyContext memory strategyContext,
195:         uint256 bptAmount
196:     ) internal view returns (uint256 primaryAmount) {
197:         (
198:            uint256 virtualSupply, 
199:            uint256[] memory balances, 
200:            uint256 invariant
201:         ) = _getValidatedPoolData(poolContext, oracleContext, strategyContext);
202: 
203:         // NOTE: For Boosted 3 token pools, the LP token (BPT) is just another
204:         // token in the pool. So, we first use _calcTokenOutGivenExactBptIn
205:         // to calculate the value of 1 BPT. Then, we scale it to the BPT
206:         // amount to get the value in terms of the primary currency.
207:         // Use virtual total supply and zero swap fees for joins
208:         primaryAmount = StableMath._calcTokenOutGivenExactBptIn({
209:             amp: oracleContext.ampParam, 
210:             balances: balances, 
211:             tokenIndex: 0, 
212:             bptAmountIn: BalancerConstants.BALANCER_PRECISION, // 1 BPT 
213:             bptTotalSupply: virtualSupply, 
214:             swapFeePercentage: 0, 
215:             currentInvariant: invariant
216:         });
217: 
218:         uint256 primaryPrecision = 10 ** poolContext.basePool.primaryDecimals;
219:         primaryAmount = (primaryAmount * bptAmount * primaryPrecision) / BalancerConstants.BALANCER_PRECISION_SQUARED;
220:     }

The virtualSupply is computed within the Boosted3TokenPoolUtils._getVirtualSupply function. Note that the oracleContext.bptBalance is used during the calculation.

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

File: Boosted3TokenPoolUtils.sol
136:     function _getVirtualSupply(
137:         ThreeTokenPoolContext memory poolContext, 
138:         BoostedOracleContext memory oracleContext
139:     ) internal pure returns (uint256 virtualSupply) {
140:         // The initial amount of BPT pre-minted is _MAX_TOKEN_BALANCE and it goes entirely to the pool balance in the
141:         // vault. So the virtualSupply (the actual supply in circulation) is defined as:
142:         // virtualSupply = totalSupply() - (_balances[_bptIndex] - _dueProtocolFeeBptAmount)
143:         //
144:         // However, since this Pool never mints or burns BPT outside of the initial supply (except in the event of an
145:         // emergency pause), we can simply use `_MAX_TOKEN_BALANCE` instead of `totalSupply()` and save
146:         // gas.
147:         virtualSupply = _MAX_TOKEN_BALANCE - oracleContext.bptBalance + oracleContext.dueProtocolFeeBptAmount;
148:     }

The oracleContext.bptBalance is derived from the balances parameter passed into the Boosted3TokenPoolMixin._boostedOracleContext function.

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/mixins/Boosted3TokenPoolMixin.sol#L103

File: Boosted3TokenPoolMixin.sol
103:     function _boostedOracleContext(uint256[] memory balances) internal view returns (BoostedOracleContext memory) {
104:         IBoostedPool pool = IBoostedPool(address(BALANCER_POOL_TOKEN));
105: 
106:         (
107:             uint256 value,
108:             /* bool isUpdating */,
109:             /* uint256 precision */
110:         ) = pool.getAmplificationParameter();
111: 
112:         return BoostedOracleContext({
113:             ampParam: value,
114:             bptBalance: balances[BPT_INDEX],
115:             dueProtocolFeeBptAmount: pool.getDueProtocolFeeBptAmount() 
116:         });
117:     }

The balances parameter passed into the Boosted3TokenPoolMixin._boostedOracleContext function is retrived from the Deployments.BALANCER_VAULT.getPoolTokens(BALANCER_POOL_ID) function at Line 195. This is a major issue because the spot balance/reserve of the Boosted BPT token (e.g. bb-a-USD) is being used in the calculation of the virtualSupply. As such, the virtualSupply can be manipulated within a single transaction/block by buying or selling the Boosted BPT token (e.g. bb-a-USD) from the pool.

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/Boosted3TokenAuraVault.sol#L190

File: Boosted3TokenAuraVault.sol
190:     function _strategyContext() private view returns (Boosted3TokenAuraStrategyContext memory) {
191:         (
192:             /* address[] memory tokens */,
193:             uint256[] memory balances,
194:             /* uint256 lastChangeBlock */
195:         ) = Deployments.BALANCER_VAULT.getPoolTokens(BALANCER_POOL_ID);
196: 
197:         return Boosted3TokenAuraStrategyContext({
198:             poolContext: _threeTokenPoolContext(balances),
199:             oracleContext: _boostedOracleContext(balances),
200:             stakingContext: _auraStakingContext(),
201:             baseStrategy: _baseStrategyContext()
202:         });
203:     }

Note that the virtualSupply is passed to the StableMath._calcTokenOutGivenExactBptIn function at Line 213. Therefore, the returned primaryAmount amount can be manipulated within a single transaction/block. With that, a malicious user manipulates the balance returned from the _getTimeWeightedPrimaryBalance function to be larger or smaller than expected depending on the attack scenarios.

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

File: Boosted3TokenPoolUtils.sol
185:     /// @notice Gets the time-weighted primary token balance for a given bptAmount
186:     /// @dev Boosted pool can't use the Balancer oracle, using Chainlink instead
187:     /// @param poolContext pool context variables
188:     /// @param oracleContext oracle context variables
189:     /// @param bptAmount amount of balancer pool lp tokens
190:     /// @return primaryAmount primary token balance
191:     function _getTimeWeightedPrimaryBalance(
192:         ThreeTokenPoolContext memory poolContext,
193:         BoostedOracleContext memory oracleContext,
194:         StrategyContext memory strategyContext,
195:         uint256 bptAmount
196:     ) internal view returns (uint256 primaryAmount) {
197:         (
198:            uint256 virtualSupply, 
199:            uint256[] memory balances, 
200:            uint256 invariant
201:         ) = _getValidatedPoolData(poolContext, oracleContext, strategyContext);
202: 
203:         // NOTE: For Boosted 3 token pools, the LP token (BPT) is just another
204:         // token in the pool. So, we first use _calcTokenOutGivenExactBptIn
205:         // to calculate the value of 1 BPT. Then, we scale it to the BPT
206:         // amount to get the value in terms of the primary currency.
207:         // Use virtual total supply and zero swap fees for joins
208:         primaryAmount = StableMath._calcTokenOutGivenExactBptIn({
209:             amp: oracleContext.ampParam, 
210:             balances: balances, 
211:             tokenIndex: 0, 
212:             bptAmountIn: BalancerConstants.BALANCER_PRECISION, // 1 BPT 
213:             bptTotalSupply: virtualSupply, 
214:             swapFeePercentage: 0, 
215:             currentInvariant: invariant
216:         });
217: 
218:         uint256 primaryPrecision = 10 ** poolContext.basePool.primaryDecimals;
219:         primaryAmount = (primaryAmount * bptAmount * primaryPrecision) / BalancerConstants.BALANCER_PRECISION_SQUARED;
220:     }

Impact

The _getTimeWeightedPrimaryBalance function is used by many functions within the Boosted Balancer Leverage Vault. The most critical function affected by this issue would be the VaultConfiguration.calculateCollateralRatio function that relies on the _getTimeWeightedPrimaryBalance function when converting the strategy tokens to underlying value (See Boosted3TokenPoolUtils._convertStrategyToUnderlying). Therefore, the collateral ratio of a vault account can be manipulated in favor of the attacker.

For instance, an attacker can manipulate the collateral ratio to be much larger than the threshold to overborrow from Notional, or an attacker can manipulate the collateral ratio of a vault account to fall below the threshold in an attempt to pre-maturely deleverage/liquidate an account.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/pool/Boosted3TokenPoolUtils.sol#L191 https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/pool/Boosted3TokenPoolUtils.sol#L136 https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/mixins/Boosted3TokenPoolMixin.sol#L103 https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/Boosted3TokenAuraVault.sol#L190

Tool used

Manual Review

Recommendation

It is recommended to avoid using spot reserve or spot price within the _getTimeWeightedPrimaryBalance function when computing the balance. Consider leveraging Chainlink as per the source code's comment to derive the balance at Line 186

File: Boosted3TokenPoolUtils.sol
185:     /// @notice Gets the time-weighted primary token balance for a given bptAmount
186:     /// @dev Boosted pool can't use the Balancer oracle, using Chainlink instead
187:     /// @param poolContext pool context variables
188:     /// @param oracleContext oracle context variables
189:     /// @param bptAmount amount of balancer pool lp tokens
190:     /// @return primaryAmount primary token balance
191:     function _getTimeWeightedPrimaryBalance(
192:         ThreeTokenPoolContext memory poolContext,
193:         BoostedOracleContext memory oracleContext,
194:         StrategyContext memory strategyContext,
195:         uint256 bptAmount
196:     ) internal view returns (uint256 primaryAmount) {
jeffywu commented 1 year ago

@T-Woodward / @weitianjie2000

T-Woodward commented 1 year ago

Think this is not a legitimate bug, but still investigating to be sure

Evert0x commented 1 year ago

@T-Woodward comment from submitter

The report highlights that price calculation is computed using virtualSupply and other variables. The virtualSupply is derived from the Deployments.BALANCER_VAULT.getPoolTokens(BALANCER_POOL_ID) function. The API spec for this function from Balancer can be found https://dev.balancer.fi/references/contracts/apis/the-vault#getpooltokens

As you can see the Balancer's getPoolTokens function will get the spot balance of the tokens within the Balance pool. Whenever a price calculation involves the spot balance of a pool, it is no longer resistant to price manipulation because the spot balance or pool can be manipulated within a single block/transaction.

T-Woodward commented 1 year ago

Yeah but I suspect that is vacuously true. Yes, the virtual supply can be changed in one transaction - by trading on the pool. But guess what - doing so also changes the balances of the other tokens in the pool. So on a net basis there's no impact. This guy hasn't shown any proof that there is a net impact. I'm 99% sure there isn't but we're still doing more testing to prove that @Evert0x