search

sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

xiaoming90 - Malicious Users Can Settle More BPT Than Needed During Emergency Settlement #102

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

xiaoming90

high

Malicious Users Can Settle More BPT Than Needed During Emergency Settlement

Summary

Malicious users can settle more BPT than needed during emergency settlement leaving no or little BPT behind to accrue the rewards for the vault shareholders.

Vulnerability Detail

The current BPT threshold is set to 20% of the total BTP supply based on the environment file provided during the audit.

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/scripts/BalancerEnvironment.py#L41

File: BalancerEnvironment.py
40:             "oracleWindowInSeconds": 3600,
41:             "maxBalancerPoolShare": 2e3, # 20%
42:             "settlementSlippageLimitPercent": 5e6, # 5%

The maxBalancerPoolShare will be used to compute the BPT threshold of a vault.

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

File: BalancerVaultStorage.sol
60:     function _bptThreshold(StrategyVaultSettings memory strategyVaultSettings, uint256 totalBPTSupply) 
61:         internal pure returns (uint256) {
62:         return (totalBPTSupply * strategyVaultSettings.maxBalancerPoolShare) / BalancerConstants.VAULT_PERCENT_BASIS;
63:     }

When the total number of BPT held by the vault exceeds the BPT threshold, the emergency settlement will be triggered to settle excess BPT to bring the ratio back to a healthy level.

Assume the following:

Then the BPT threshold of the vault will be 20 BPT.

Based on the current design of the Notional leverage vault, if the vault holds 30 BPT, around 10 BPT will be settled during the emergency settlement to bring the total number of BPT held by the vault back to 20 BPT. The reason for not settling all the BPT within the vault is that once the number of BPT held by the vault returns back to a healthy ratio, the remaining BPT held by the vault will continue to accrue the rewards for the vault shareholders.

However, a malicious user could settle an excessive number of the BPT within the vault during the emergency settlement and leave no or little BPT behind to accrue the rewards for the vault shareholders.

Similarly, assume the following at this point in time:

This will only cause 10 BPT to be settled during emergency settlement. However, the issue is that the BPT threshold is calculated on the spot/current total supply of BPT which can be manipulated within a single block/transaction. BPT can also be obtained from the open market.

A malicious user could obtain a flash-loan DAI and obtain 90 BPT from the open market. He then exits the Balancer pool with all the 90 BPT and redeems the underlying assets, and this will cause the total supply of BPT to fall from 100 to 10.

At this point, the total supply of BPT is 10, so the BPT threshold of the vault will become 2 BPT (10 * 0.2). The malicious user proceeds to trigger the emergency settlement, and the formula determines that 28 BPT (30 - 2) needs to be settled to bring the number of BPT held by the vault to a healthy ratio. After the emergency settlement, the vault is only left with 2 BPT to accrue the rewards for the vault shareholders.

After the attack, the attacker proceeds to swap the redeemed underlying assets to the DAI and repay back the loan. Flash-loan cost is 1 wei in dydx, so the cost of attack is limited to slippage during swaps. If swaps involve only stablecoin, then the cost of attack is further reduced since stable swap usually has minimum slippage.

Line 65 below shows that the totalBPTSupply used for computing the BPT threshold is the spot/current total supply of BPT of a pool.

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/external/MetaStable2TokenAuraHelper.sol#L52

File: MetaStable2TokenAuraHelper.sol
52:     function settleVaultEmergency(
53:         MetaStable2TokenAuraStrategyContext calldata context, 
54:         uint256 maturity, 
55:         bytes calldata data
56:     ) external {
57:         RedeemParams memory params = SettlementUtils._decodeParamsAndValidate(
58:             context.baseStrategy.vaultSettings.emergencySettlementSlippageLimitPercent,
59:             data
60:         );
61: 
62:         uint256 bptToSettle = context.baseStrategy._getEmergencySettlementParams({
63:             poolContext: context.poolContext.basePool, 
64:             maturity: maturity, 
65:             totalBPTSupply: IERC20(context.poolContext.basePool.pool).totalSupply()
66:         });
67: 
68:         uint256 redeemStrategyTokenAmount = 
69:             context.baseStrategy._convertBPTClaimToStrategyTokens(bptToSettle);
70: 
71:         _executeSettlement({
72:             strategyContext: context.baseStrategy,
73:             oracleContext: context.oracleContext,
74:             poolContext: context.poolContext,
75:             maturity: maturity,
76:             bptToSettle: bptToSettle,
77:             redeemStrategyTokenAmount: redeemStrategyTokenAmount,
78:             params: params
79:         });
80: 
81:         emit BalancerEvents.EmergencyVaultSettlement(maturity, bptToSettle, redeemStrategyTokenAmount);
82:     }

Impact

A malicious user could settle an excessive number of BPT within the vault during the emergency settlement and leave no or little BPT behind to accrue the rewards for the vault shareholders. Loss of assets for the vault shareholders.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/scripts/BalancerEnvironment.py#L41 https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/BalancerVaultStorage.sol#L60 https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/external/MetaStable2TokenAuraHelper.sol#L52

Tool used

Manual Review

Recommendation

Consider updating the implementation to avoid using the spot/current total supply of BPT to determine the BPT threshold as this can be manipulated within a single block/transaction to force a certain condition. A short-term quick fix would be to hardcode the threshold or have the BTP threshold manually set by Notional governance instead of computing the threshold on the fly.

In addition, the emergency settlement function is permissionless and can be called by anyone. If possible, implement access control to ensure that this function can only be triggered by Notional to reduce the attack surface.

duplicate of #66

jeffywu commented 1 year ago

This is a duplicate of #66 as it is a side effect of an improper emergency settlement.

See my comment in #66, I'm not sure there would be a massive loss of value beyond the slippage boundaries set by governance to other vault holders. They would lose future returns but not existing funds.

jeffywu commented 1 year ago

Similar to the other issue, I think these two issues are duplicates based on the underlying cause but will defer to the judging criteria if duplicates should be based on the effect of the a certain issue.

My opinion here is that if we use side effects as the basis for uniqueness in issues, then for a basic level issue in an internal accounting method could spawn many potential side effects which could be all classified as "internal account issues". Each auditor who enumerates a unique side effect would be considered a unique issue. That doesn't seem like the right decision to me.