Closed sherlock-admin closed 6 months ago
Invalid, known issue
The PRICE submodules that use an on-chain method to access the reserves of a liquidity pool or positions are susceptible to sandwich attacks and multi-block manipulation - Assets in PRICEv2 can be configured to track the moving average of an asset price in order to mitigate this risk - Assets in PRICEv2 can be configured with multiple price feeds and a reconciliation strategy (e.g. average, median, average if a deviation is present) in order to mitigate this risk - Where possible, PRICE submodules will check for re-entrancy in the source (e.g. liquidity pool). This has been implemented in the BalancerPoolTokenPrice and Uniswap submodule.
The SPPLY submodules that use an on-chain method to access the reserves of a liquidity pool or positions are susceptible to sandwich attacks and multi-block manipulation - Where possible, downstream consumers of the data need to conduct sanity-checks. - Where possible, SPPLY submodules will check for re-entrancy in the source (e.g. liquidity pool). This has been implemented in the AuraBalancerSupply submodule. - To guard against multi-block manipulation, where possible, SPPLY submodules will compare the implied price from reserves against the price from TWAP. This has been implemented in the BunniSupply submodule.
Escalate
Hey @nevillehuang - I think there's a bit of misunderstanding here. The point of the issue is that the logic to calculate LP price of BunniTokens is incorrect. As shown in the codebase, it calculates the price of BunniTokens as what Warp Finance did(value of assets divided by number of LPs), which is incorrect. I've added a link Pricing LP tokens for better understanding.
Escalate
Hey @nevillehuang - I think there's a bit of misunderstanding here. The point of the issue is that the logic to calculate LP price of BunniTokens is incorrect. As shown in the codebase, it calculates the price of BunniTokens as what Warp Finance did(value of assets divided by number of LPs), which is incorrect. I've added a link Pricing LP tokens for better understanding.
You've created a valid escalation!
To remove the escalation from consideration: Delete your comment.
You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.
@KupiaSecAdmin there is no misunderstanding, the issue you highlighted is dependent on manipulating reserves of uniswap pools, which is a known issue as I highlighted. This is allowed because there are many mitigations in place to counter this issue, as known by this comments here by sponsor.
@nevillehuang - This is not about manipulating uniswap reserves, what I meant is the logic itself to calculate BunniToken price is wrong.
@0xJem you might want to take a look at this, however I am not convinced and believe this is expected behavior:
In the blog post under the section "Warp Finance Hack", the third step has shown it directly involves manipulating the reserves. As I mentioned above, olympus has a range of factors in place to mitigate this, thats why it is considered an accepted risk.
- Swap the remainder of the flash loan in the same DAI <> WETH pool. This changes the LP reserves and leads to Warp Finance using a wrong LP price.
I don't believe this is valid.
We acknowledge the potential for incorrect pricing when using the reserves of a full-range position to calculate the LP position price. For this reason, there are mitigations in place:
Reconsidered, it seems like the deviation check will not be passed. Thanks for the clarification guys! @0xJem @nevillehuang 👍
Since the submitter agreed with invalidation, planning to execute on it.
Result: Invalid Has duplicates
KupiaSec
high
Invalid price calculation for BunniTokens leads to price manipulation
Summary
BunniToken price is calculated using price of reserved tokens in the pool, leads to easy price manipulation as Warp Finance has been attacked. REF: Pricing LP tokens
Vulnerability Detail
BunniPrice
submodule only works with BunniTokens with full-range positions. It's not validated directly but it's guaranteed by checking deviations.For non full-range positions,
reservesTokenRatio
is not even similar totwapTokenRatio
. Thus, these full-range positions on UniswapV3 works as same as UniswapV2 pools.However, in calculating BunniToken price(aka UniswapV3 LP price), it sums up the price of token reserves in the pool:
Calculating the BunniToken price using this formula includes the vulnerability which is described in the reference link above(Warp Finance hack with price manipulation).
Impact
BunniToken price can be manipulated by an attacker to generate profits from the vulnerability.
Code Snippet
https://github.com/sherlock-audit/2023-11-olympus/blob/9c8df76dc9820b4c6605d2e1e6d87dcfa9e50070/bophades/src/modules/PRICE/submodules/feeds/BunniPrice.sol#L162-L165 https://github.com/sherlock-audit/2023-11-olympus/blob/9c8df76dc9820b4c6605d2e1e6d87dcfa9e50070/bophades/src/modules/PRICE/submodules/feeds/BunniPrice.sol#L215-L233
Tool used
Manual Review
Recommendation
As in the above reference link, fair LP price calculation is introduced as follows: $$ p(LP) = \frac{2 \sqrt{p0 p1 k}}{L}, k=x y $$ For UniswapV3, $L = \sqrt{x y}$, so we can rewrite the above formula like $$ p(LP) = \frac{2 \sqrt{p0 p1 L^2}}{L} = 2 \sqrt{p0 p1} $$