Steadefi

Steadefi
DeFiHardhatFoundryOracle
35,000 USDC
View results
Submission Details
Severity: low
Invalid

Refund Fees will be sent to

Summary

After Deposits To GMX Pool extra funds is sent to refundee which is msg.sender. Which won't work for EIP4337 [Account's Abstraction] .

Vulnerability Details

Analysing the current mass adoption of account abstraction we need to make sure the current logic is compatible for such users.

File: GMXDeposit.sol
/**
* @notice @inheritdoc GMXVault
* @param self GMXTypes.Store
* @param isNative Boolean as to whether user is depositing native asset (e.g. ETH, AVAX, etc.)
*/
function deposit(
GMXTypes.Store storage self,
GMXTypes.DepositParams memory dp,
bool isNative
) external {
// Sweep any tokenA/B in vault to the temporary trove for future compouding and to prevent
// it from being considered as part of depositor's assets
if (self.tokenA.balanceOf(address(this)) > 0) {
self.tokenA.safeTransfer(self.trove, self.tokenA.balanceOf(address(this)));
}
if (self.tokenB.balanceOf(address(this)) > 0) {
self.tokenB.safeTransfer(self.trove, self.tokenB.balanceOf(address(this)));
}
self.refundee = payable(msg.sender);
[............]
}

https://github.com/Cyfrin/2023-10-SteadeFi/blob/main/contracts/strategy/gmx/GMXDeposit.sol#L54

refundee has been set to msg.sender. But in the case user is using account's abstraction msg.sender will be bundler which executes the actual tx not the actual signer of tx.

So adding a receiver in deposit param could make this logic executable for such cases. User can make create useroperation with a receiver address and that address can be used as refundee in the current logic.

Even current logic will break if the user is blacklisted by the token so his funds will stuck in the protocol, Adding a redundee for withdrawl the token always work best

Impact

  1. Accounts abstraction is used by mass users and protocol is not compatible for such users.

  2. User's funds can stuck in the prtocol if user is blacklisted by the token

Tools Used

Manual review

Recommendations

Consider adding receiver param in deposit params as well as in withdraw params

Updates

Lead Judging Commences

hans Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

INFO: Consider supporting account abstraction

Support

FAQs

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