_mint() relies on current collateralToken and indexToken balances to calculate how many shares need to be issued to the depositor:
and
This can be exploited by an attacker. They can donate some collateral tokens to the vault such that totalAmountBefore
is inflated and _shares = amount * totalShares / totalAmountBefore
evaluates to zero (or at least negatively impacted). This donation would happen before the afterOrderExecution()
callback from GMX gets executed.
Please refer PoC section for a concrete example. Note that it's quite tricky to change the config params of the test suite hence the PoC sticks to the standard deposit amount of 100 USDC
and the configured market prices. This results in the donation amount to come out to be a large figure. However for collateral tokens with 18 decimal precision and different market prices, this goes down significantly. Alternatively, attacker can choose to donate index tokens instead of collateral tokens, which would have the same effect.
Attack Path:
Attacker has a position in the vault and owns some shares.
A position is opened by the vault.
Alice the naive user deposits some collateral tokens.
During the deposit flow, Keeper calls runNextAction()
.
After this, we wait for the GMX callback to afterOrderExecution()
.
Attacker front-runs this callback (or back-runs step 4's runNextAction()
) with a donation. Note that the attacker DOES NOT need to monitor the mempool (which is private) in any manner. Attacker can simply monitor on-chain txs and the moment they see runNextAction()
is executed, they donate. Since the callback from GMX is likely to have some time lag, this would work.
afterOrderExecution()
gets called now which calls _mint()
. Due to the donation, zero shares are issued to Alice even though funds have been deposited.
Attacker can now withdraw their shares. Since withdrawal too depends on current balance of the vault, Alice's funds are siphoned off to the attacker.
Depositor's funds can be stolen by an attacker. Depositor receives less shares than expected.
Add these two tests inside test/PerpetualVault.t.sol
and run via forge test --mt test_donationBug_Pass -vv --via-ir --rpc-url arbitrum
. The first test, test_donationBug_Pass_1_someSharesMinted()
shows the normal path where some shares are minted to the depositor. The second test, test_donationBug_Pass_2_zeroSharesMinted()
shows how no shares are minted after an attacker's donation:
It would be better to track balances internally so that they can't be manipulated by donations.
Some initial "dead" shares can be minted to the protocol to make this attack unprofitable.
Set a minShares
param which must be issued during deposits.
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.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.