USDC is a well known token that has a fee switch that could be activated at any time.
If this happens, any PerpetualVault using it will be broken on deposits, as the amount of token used to mint shares do not account for fees.
The issue will cause total deposited amount and users deposit amounts to be wrongly accounted:
Every time deposit is called the totalDepositAmount will be wrong
For users deposit, on first deposit, this one is the less inconvenient as its very unlikely to happen
on any deposits made in a vault with an open position, so on most of the deposits
I've listed some situations where this will cause wrong accounting, but there might be others.
During first deposit, we see L230 that the totalDepositedAmount is incremented by amount, and depositInfo[counter] initialized with amount too, while when FoT are in play, the real value received by the vault will be in reality amount - someFee.
We also see L235 that the deposit function will directly call the _mint function (as positionIsClosed is set to true on initialization), which will then mint to the user an amount of shares equal to depositInfo[depositId].amount * 1e8 where depositInfo[depositId].amount == amount.
Once the vault has received deposits and opened a position, the deposit function will not directly mint shares to the users, but rather set the nextAction.selector to INCREASE_ACTION.
This will trigger keepers into calling the runNextAction(), which will then call _createIncreasePosition in case of a vault with leverage >1, creating a GMX order.
Once the GMX order has been successfully created, the afterOrderExecution will be called, and the FLOW.DEPOSIT case will be executed.
We then see, L484 that the amount that will be used to mint shares to the users is read again from depositInfo[counter].amount, which as we shown before that, do not account for FoT.
Finally, that amount is used to compute increased at L490 (where feeAmount are GMX fees), and fed to the _mint function L494.
Wrong accounting for USDC if fees are activated.
Likelihood: low, as this require USDC to activate fees and we don't know when this will happens
Impact: high, as this would break accounting for the vault
Rather than relying of the amount input in deposit(), measure the actual received amount by comparing balanceBefore and balanceAfter.
Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point. Keepers are added by the admin, there is no "malicious keeper" and if there is a problem in those keepers, that's out of scope. ReadMe and known issues states: " * System relies heavily on keeper for executing trades * Single keeper point of failure if not properly distributed * Malicious keeper could potentially front-run or delay transactions * Assume that Keeper will always have enough gas to execute transactions. There is a pay execution fee function, but the assumption should be that there's more than enough gas to cover transaction failures, retries, etc * There are two spot swap functionalies: (1) using GMX swap and (2) using Paraswap. We can assume that any swap failure will be retried until success. " " * Heavy dependency on GMX protocol functioning correctly * Owner can update GMX-related addresses * Changes in GMX protocol could impact system operations * We can assume that the GMX keeper won't misbehave, delay, or go offline. " "Issues related to GMX Keepers being DOS'd or losing functionality would be considered invalid."
There is no real proof, concrete root cause, specific impact, or enough details in those submissions. Examples include: "It could happen" without specifying when, "If this impossible case happens," "Unexpected behavior," etc. Make a Proof of Concept (PoC) using external functions and realistic parameters. Do not test only the internal function where you think you found something.
Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point. Keepers are added by the admin, there is no "malicious keeper" and if there is a problem in those keepers, that's out of scope. ReadMe and known issues states: " * System relies heavily on keeper for executing trades * Single keeper point of failure if not properly distributed * Malicious keeper could potentially front-run or delay transactions * Assume that Keeper will always have enough gas to execute transactions. There is a pay execution fee function, but the assumption should be that there's more than enough gas to cover transaction failures, retries, etc * There are two spot swap functionalies: (1) using GMX swap and (2) using Paraswap. We can assume that any swap failure will be retried until success. " " * Heavy dependency on GMX protocol functioning correctly * Owner can update GMX-related addresses * Changes in GMX protocol could impact system operations * We can assume that the GMX keeper won't misbehave, delay, or go offline. " "Issues related to GMX Keepers being DOS'd or losing functionality would be considered invalid."
There is no real proof, concrete root cause, specific impact, or enough details in those submissions. Examples include: "It could happen" without specifying when, "If this impossible case happens," "Unexpected behavior," etc. Make a Proof of Concept (PoC) using external functions and realistic parameters. Do not test only the internal function where you think you found something.
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.