jacksanford1
commented
1 year ago Adding some commentary discussed in other forums:
0x52
There are a few aspects to this.
First the deposit would have to have a large enough amount of USDC in the senior to fully hedge such a large deposit.
Two concerns if there was a big deposit
1) slippage bounds on hedge, which is what I believe you're pointing out
2) deposit cap on junior
Technically both could be fixed with parameter changes. i.e. allow higher slippage, deposit then lower slippage or increasing deposit cap
Probably a good idea to make sure you block USDC deposits on batching when its close to the deposit cap to avoid that (i.e. stop taking deposits when you get to 98% of deposit cap on junior)
Not sure there's really a good way to prevent the slippage issue though. Potentially you could add an admin override function to make smaller batch deposits if it gets too large Another DOS would be for someone to reach deposit cap before glp was staked to lock it because TotalAssets doesn't account for USDC in batchingManager No permanent damage but might push the deposit higher than you want As an example if it's at 90% of deposit cap and users deposit 10% of deposit cap as USDC to batching manager. Before it's staked to GLP, someone could make a small deposit to the junior and now when the batchingmanager tries to deposit it will revert because it goes over the cap
Rage Trade
Hey 0x52 we have created a potential solution for this: 1) To solve the slippage issue we have added possibility of splitting batch deposit into multiple steps 2) To solve the deposit cap issue we have added deposit cap into batching manager to ensure that more than certain amount of USDC is not added In case it breaches we could increase cap for vault to allow for batching manger deposit and make the cap on batching manger 0 to prevent any deposits in the next round. Here is the link to PR: https://github.com/RageTrade/delta-neutral-gmx-vaults/pull/51
IAm0x52
0x52
RageTrade (found by protocol team after contest)
medium
Denial of Service on Batching Manager
Summary
In case there is significant amount (like $1M) of USDC deposited into Batching Manager then after conversion of usdc into sGLP would work fine (executeBatchStake) but from sGLP to shares (executeBatchDeposit) would fail due to slippage tolerance checks. This would lead to batching manager being stuck in this state since batch deposit cannot go through and users would not be able to withdraw the sGLP.
Vulnerability Detail
Impact
Users would not be able to withdraw the sGLP from the Batching Manager.
Code Snippet
https://github.com/sherlock-audit/2022-10-rage-trade/blob/main/dn-gmx-vaults/contracts/vaults/DnGmxBatchingManager.sol#247
Tool used
Manual Review
Recommendation
Fixed by RageTrade in PR #51