Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test case for proposed attack vector (uniswapDeposit sandwich) #213

Merged
merged 2 commits into from
Nov 12, 2023

Conversation

haydenshively
Copy link
Member

@haydenshively haydenshively commented Nov 1, 2023

This issue proposes an attack vector where a uniswapDeposit (or other Uni V3 mint) is sandwiched between two swaps -- one that pushes away from the TWAP, and another that moves back towards it. More specifically, a Borrower's borrowed assets are used to create a very thin range order (limit order) just below the manipulated price, causing it to buy ETH at far higher than fair value (2x in the example given).

Assuming a USDC/WETH pool for simplicity, a naive health check implementation could look like this:

(uint256 usdc, uint256 eth) = LiquidityAmounts.getAmountsForLiquidity(
    slot0.sqrtPriceX96,
    sqrtLower,
    sqrtUpper,
    liquidity
)
uint256 valueInTermsOfETH = usdc * TWAP + eth;

The position's value is calculated using the TWAP. This is good. The danger is that underlying amounts are determined by the instantaneous slot0.sqrtPriceX96. This allows an attacker to manipulate the (uint256 usdc, uint256 eth) tuple so that it swings from (1M USDC, 0 ETH) to (0 USDC, 1M / X ETH) for arbitrary values of X. Essentially, the protocol has assumed X≅TWAP when it's not.

Aloe II does not make this mistake. Our code looks something like this:

uint256 sqrtProbePrice = sqrtTWAP * k; // or `sqrtTWAP / k` for the lower probe price
(uint256 usdc, uint256 eth) = LiquidityAmounts.getAmountsForLiquidity(
    sqrtProbePrice,
    sqrtLower,
    sqrtUpper,
    liquidity
)
uint256 valueInTermsOfETH = usdc * (sqrtProbePrice ** 2) + eth;

The key is that we (a) find underlying amounts and (b) combine them to get total value at the same price (sqrtProbePrice). And we do not use the instantaneous slot0 variables for this at all. Getting back to the {buys ETH at twice its fair price} example: that can certainly happen in a Borrower, and the Borrower would certainly lose money. But our health check accounts for that hypothetical loss before it happens such that atomically-dangerous positions cannot be constructed.

This new test simply demonstrates that.

Spreadsheet with results for various sandwich conditions:
https://docs.google.com/spreadsheets/d/11TUjflXETGLKv3VuI0OcLYEpbi-2b2ipGfvS8cISMbA/edit?usp=sharing

@haydenshively haydenshively changed the base branch from master to sherlock-audit-fixes November 8, 2023 20:53
@haydenshively haydenshively merged commit fb649b1 into sherlock-audit-fixes Nov 12, 2023
3 of 4 checks passed
@haydenshively haydenshively deleted the test-uniswap-mint-attack branch November 12, 2023 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant