hats-finance / Possum-Labs--Portals--0xed8965d49b8aeca763447d56e6da7f4e0506b2d3

GNU General Public License v2.0
0 stars 2 forks source link

Users who have more portalEnergy than maxStakeDebt stand to lose their excess portalEnergy tokens if they call ForceUnstakeAll #33

Open hats-bug-reporter[bot] opened 11 months ago

hats-bug-reporter[bot] commented 11 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x7f7b4d7433c296008670e9150291f5bea0cf93f38bdfab335460f451079d0c67 Severity: high

Description: Description\ Users who have acquired more portalEnergy than maxStakeDebt by specifically purchasing portalEnergy stand to lose their excess portalEnergy tokens if they call ForceUnstakeAll. This is because ForceUnstakeAll does not take into account a situation where portalEnergy is more than their maxStakeDebt.

Attack Scenario\ Describe how the vulnerability can be exploited.

  1. User calls BuyPortalEnergy to purchase more portalEnergy. This causes their portalEnergy to be more than maxStakeDebt.
  2. User wishes to close their stake and calls ForceUnstakeAll.

Attachments

  1. Proof of Concept (PoC) File

    
    function forceUnstakeAll() external nonReentrant existingAccount {
        /// @dev Update the user's stake data
        _updateAccount(msg.sender,0);
    
        /// @dev Initialize cached variable
        uint256 portalEnergy = accounts[msg.sender].portalEnergy;
    
        /// @dev Calculate how many portalEnergyToken must be burned from the user's wallet, if any
        if(portalEnergy < accounts[msg.sender].maxStakeDebt) {
    
            uint256 remainingDebt = accounts[msg.sender].maxStakeDebt - portalEnergy;
    
            /// @dev Require that the user has enough Portal Energy Tokens
            if(IERC20(portalEnergyToken).balanceOf(address(msg.sender)) < remainingDebt) {revert InsufficientPEtokens();}
    
            /// @dev Burn the appropriate portalEnergyToken from the user's wallet to increase portalEnergy sufficiently
            _burnPortalEnergyToken(msg.sender, remainingDebt);
        }
    
        /// @dev Withdraw the principal from the yield source to pay the user
        uint256 balance = accounts[msg.sender].stakedBalance;
        _withdrawFromYieldSource(balance);
    
        /// @dev Update the user's stake info
        accounts[msg.sender].stakedBalance = 0;
        accounts[msg.sender].maxStakeDebt = 0;
        portalEnergy = accounts[msg.sender].portalEnergy -= (balance * maxLockDuration) / SECONDS_PER_YEAR;
        accounts[msg.sender].availableToWithdraw = 0;
    
        /// @dev Send the user´s staked balance to the user
        IERC20(PRINCIPAL_TOKEN_ADDRESS).safeTransfer(msg.sender, balance);
    
        /// @dev Update the global tracker of staked principal
        totalPrincipalStaked -= balance;
    
        /// @dev Emit an event with the updated stake information
        emit StakePositionUpdated(msg.sender, 
        block.timestamp,
        maxLockDuration,
        0,
        0, 
        portalEnergy,
        0);
    }
Add this code at Ln 361:

if(portalEnergy > accounts[msg.sender].maxStakeDebt) {

uint256 remainingDebt = portalEnergy - accounts[msg.sender].maxStakeDebt ;

/// @dev Mint to user wallet

portalEnergyToken.mint(msg.sender, remainingDebt);
accounts[msg.sender].portalEnergy -= remainingDebt;

}


2. **Revised Code File (Optional)**
<!-- If possible, please provide a second file containing the revised code that offers a potential fix for the vulnerability. This file should include the following information:
- Comment with a clear explanation of the proposed fix.
- The revised code with your suggested changes.
- Any additional comments or explanations that clarify how the fix addresses the vulnerability. -->
PossumLabsCrypto commented 11 months ago

Invalid.

As you can see in the following section, forceUnstakeAll() considers the scenario where the portalEnergy balance is higher than maxStakeDebt, hence not setting portalEnergy to zero.

/// @dev Update the user's stake info accounts[msg.sender].stakedBalance = 0; accounts[msg.sender].maxStakeDebt = 0; portalEnergy = accounts[msg.sender].portalEnergy -= (balance * maxLockDuration) / SECONDS_PER_YEAR; accounts[msg.sender].availableToWithdraw = 0;

The user will maintain Portal Energy as internal balance if he has a surplus after unstaking. This surplus balance can be sold into the internal LP or minted as ERC20 token, hence is fully available by the user even after unstaking.