Steadefi

Summary

Referencing used in GMXEmergency.sol#emergencyWithdraw is wrong. Under emergency condition, User calls the emergencyWithdraw function from GMXVault.sol , which calls GMXEmergency#emergencyWithdraw. Here for the call to GMXVault.sol, the msg.sender is User but for the call to GMXEmergency#emergencyWithdraw , the msg.sender is GMXVault.sol and not User. So while burning Vault tokens, we need to properly reference it to User but the GMXEmergency has referenced it to GMXVault.sol .

Vulnerability Details

Let's first look at the processDeposit function, here after depositing funds, the Vault shares are minted to User as shown below:
https://github.com/Cyfrin/2023-10-SteadeFi/blob/main/contracts/strategy/gmx/GMXDeposit.sol#L172

Similar referencing is done while withdrawing the tokens at :
https://github.com/Cyfrin/2023-10-SteadeFi/blob/main/contracts/strategy/gmx/GMXWithdraw.sol#L197

But during the Emergency withdrawal, the referencing done is wrong. The flow of emergency withdrawal process is :
USER calls GMXVault.sol , where msg.sender is USER and GMXVault.sol in turn calls GMXEmergency.sol#emergencyWithdraw where the msg.sender is GMXVault.sol . The problem occurs while burning of Vault tokens at :
https://github.com/Cyfrin/2023-10-SteadeFi/blob/main/contracts/strategy/gmx/GMXEmergency.sol#L182

here, burning is done referencing msg.sender which is assumed to be USER , but in reality it is GMXVault.sol.

Impact

Complete DOS of emergency withdrawal functionality.

Tools Used

Manual Review

Recommendations

Use correct referencing of the USER as done in deposit and withdraw functions.