_withdrawFromYieldSource might withdraw less than the actual requested amount which would be a direct loss for the Portal contract, which is a Protocol insolvency issue

[hats-bug-reporter[bot]]

Github username: @dontonka Twitter username: 0xDontonka Submission hash (on-chain): 0x16e69d228a69ff099a200fd6f9f38bf32ae518f71e3f4bddce1dd3e365e4d6b4 Severity: high

Description: Description\ _withdrawFromYieldSource called during unstake is interacting with a 3rd party contract which is earning yield. While the current HLP_STAKING::withdraw (0xbE8f8AF5953869222eA8D39F1Be9d03766010B1C) seems safe and reverts in case the full requested amount is not withdrawn to the Portal contract, it's an upgradable contract, so that behavior can always change in the future at any moment. Furthermore, this might not be the case in the future when the Portal use other 3rd party to earn yield.

Attack Scenario\ A user just unstake as usual amount X but somehow the 3rd app earning the yield does return less then the requested amount (X - y), but since no verification is made in Portal (which assume and fully trust the 3rd party yield app, which is the flaw here), all the maths in Portal works, and the user balance is substracted for the full amount X, not the real amount (X - y), and the Portal is sending the real amount to the user.

The problem now is that the Portal has incurr a loss for the amount (y), which will make it insolvent at some point.

Attachments

1. Proof of Concept (PoC) File

   • Alice stake 100 token in Portal.
   • After few days, she change her mind and decide to unstake her 100 token by calling unstake.
   • The transaction revert as the protocol doesn't have enought funds since it has been suffering the exposed problem multiples times and is running out of funds (insolvency).

2. Revised Code File (Optional)

   
   function unstake(uint256 _amount) external nonReentrant existingAccount {
       /// @dev Update the user's stake data
       _updateAccount(msg.sender,0);
   
       /// @dev Require that the amount to be unstaked is less than or equal to the available withdrawable balance and the staked balance
       if(_amount > accounts[msg.sender].availableToWithdraw) {revert InsufficientToWithdraw();}
       if(_amount > accounts[msg.sender].stakedBalance) {revert InsufficientStake();}
   
       /// @dev Withdraw the matching amount of principal from the yield source (external protocol)
   +       uint256n balanceBefore = IERC20(PRINCIPAL_TOKEN_ADDRESS).balanceOf(address(this));        
       _withdrawFromYieldSource(_amount);
   +       uint256n balanceAfter = IERC20(PRINCIPAL_TOKEN_ADDRESS).balanceOf(address(this));
   +       _amount = balanceAfter - balanceBefore;
   
       /// @dev Update the user's stake info & cache to memory
       uint256 stakedBalance = accounts[msg.sender].stakedBalance -= _amount;
       uint256 maxStakeDebt = accounts[msg.sender].maxStakeDebt -= (_amount * maxLockDuration) / SECONDS_PER_YEAR;
       uint256 portalEnergy = accounts[msg.sender].portalEnergy -= (_amount * maxLockDuration) / SECONDS_PER_YEAR;
       uint256 availableToWithdraw = accounts[msg.sender].availableToWithdraw -= _amount;
   
       /// @dev Update the global tracker of staked principal
       totalPrincipalStaked -= _amount;
   
       /// @dev Send the principal tokens to the user
       IERC20(PRINCIPAL_TOKEN_ADDRESS).safeTransfer(msg.sender, _amount);
   
       /// @dev Emit an event with the updated stake information
       emit StakePositionUpdated(msg.sender, 
       block.timestamp,
       maxLockDuration,
       stakedBalance,
       maxStakeDebt, 
       portalEnergy,
       availableToWithdraw);
   }

[PossumLabsCrypto]

Yes, the Portal is dependent on the underlying protocol. In the case where the underlying protocol uses proxy contracts, there is always the danger that the Portal gets "rugged" one way or another.

The provided fix does not change anything about the underlying trust assumptions and therefore cannot mitigate a potential Portal insolvency / loss of user funds caused by a malicious change in the HLP proxy.

To my understanding, this is an attack/vulnerability that requires "Access to leaked private keys or trusted addresses" and is therefore out of scope according to the competition rules.

However, I want to personally acknowledge this finding and reward you outside of the competition budget.

I reach out to you on twitter.