sherlock-audit / 2023-03-taurus-judging

4 stars 0 forks source link

GimelSec - `BaseVault.emergencyClosePosition` should still take the reward into consideration. #157

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

GimelSec

high

BaseVault.emergencyClosePosition should still take the reward into consideration.

Summary

BaseVault.emergencyClosePosition should have the modifier updateReward. Or the user could suffer from loss of funds.

Vulnerability Detail

BaseVault.emergencyClosePosition should have the modifier updateReward. Even the protocol is paused. There is no reason that the users cannot take back their rewards. https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/BaseVault.sol#L227

    function emergencyClosePosition() external {
        _modifyPosition(msg.sender, userDetails[msg.sender].collateral, userDetails[msg.sender].debt, false, false);
    }

Impact

The users cannot take back their rewards when the protocol is paused.

Code Snippet

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/BaseVault.sol#L227 https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/BaseVault.sol#L78

Tool used

Manual Review

Recommendation

Add updateReward modifier on emergencyClosePosition()

    function emergencyClosePosition() external updateReward(msg.sender) {
        _modifyPosition(msg.sender, userDetails[msg.sender].collateral, userDetails[msg.sender].debt, false, false);
    }

Duplicate of #117