Submission Details
Severity:
Unnecessary `payable` in `GMXVault.deposit()`
Summary
There is a payble function in the GMXVault.deposit(), but msg.value isn't used - therefore the function payable state modifier isn't necessary.
It's danger for users if they send ETH by mistake.
Vulnerability Details
// file: contracts/strategy/gmx/GMXVault.sol#L301-L303
function deposit(GMXTypes.DepositParams memory dp) external payable nonReentrant {
GMXDeposit.deposit(_store, dp, false); // @audit remove payable
}
// file: contracts/strategy/gmx/GMXDeposit.sol#L78-L84
function deposit(
GMXTypes.Store storage self,
GMXTypes.DepositParams memory dp,
bool isNative
) external {
//...
if (isNative) {
GMXChecks.beforeNativeDepositChecks(self, dp);
self.WNT.deposit{ value: dp.amt }();
} else {
IERC20(dp.token).safeTransferFrom(msg.sender, address(this), dp.amt);
}
//...
}
Impact
This could lead to users losing the eth sent with this transaction, which would be better served by reverting instead of accepting a non-zero value of msg.value
Tools Used
Manual Review
Recommendations
Simple fix, remove the payable in GMXVault.deposit().
Prize pool breakdown
Total prize
35,000 USDC
nSLOC
2,28915 USDC / LOC
33,000 USDC
2,000 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.