Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Passing in ETH/fee when Position is closed will cause funds to be stuck in the perp vault

bigsam

Summary

Deposit is marked as payable to take in fee to process orders passing in msg.value when the vault is Closed will cause the fee to be stuck in the Perpvault.

Vulnerability Details

See below

function deposit(uint256 amount) external nonReentrant payable { // marked as payable but do we expect ETH????
_noneFlow();
if (depositPaused == true) {
revert Error.Paused();
}
if (amount < minDepositAmount) {
revert Error.InsufficientAmount();
}
if (totalDepositAmount + amount > maxDepositAmount) {
revert Error.ExceedMaxDepositCap();
}
flow = FLOW.DEPOSIT;
collateralToken.safeTransferFrom(msg.sender, address(this), amount);
counter++;
depositInfo[counter] = DepositInfo(amount, 0, msg.sender, 0, block.timestamp, address(0)); // we can just save the deadline since locktime can be extended or reduced
totalDepositAmount += amount; // calculate shares based on this and scale up ...... amount deposited/total amount/ now i understand why we use address this
EnumerableSet.add(userDeposits[msg.sender], counter);
​
@audit>> if (positionIsClosed) { // if user passes in msg.value they will lose it and it will not be refunded ..... try something else LOW user error
@audit>> MarketPrices memory prices;
@audit>> _mint(counter, amount, false, prices); // no refund
@audit>> _finalize(hex'');

The pay execution fee directly forward the fee from the user to the GMX proxy for use

/**
* @notice
* Any excess execution fee from GMX call is not refunded to the user.
*/
function _payExecutionFee(uint256 depositId, bool isDeposit) internal {
uint256 minExecutionFee = getExecutionGasLimit(isDeposit) * tx.gasprice;
​
if (msg.value < minExecutionFee) {
revert Error.InsufficientAmount();
}
if (msg.value > 0) {
payable(address(gmxProxy)).transfer(msg.value);
depositInfo[depositId].executionFee = msg.value;
}
}

Hence there is a need to prevent msg.value from being passed into a closed vault as it is not retrievable

Impact

Stucked fee in Perp Vault

Tools Used

Manual Review

Recommendations

Nest a check to revert deposits that pass in msg.value when the vault is closed.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas

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.

Users mistake, only impacting themselves.

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.

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas

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.

Users mistake, only impacting themselves.

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.

Support

FAQs

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

Give us feedback!