c4-bot-9 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PowerFarms/PendlePowerFarmController/PendlePowerFarmController.sol#L53-L111 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PowerFarms/PendlePowerFarmController/PendlePowerFarmToken.sol#L452-L463 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PowerFarms/PendlePowerFarmController/PendlePowerFarmToken.sol#L465-L476

Vulnerability details

Impact

In certain scenarios, shares of a subsequent depositor can be heavily reduced, losing a large amount of his deposited funds, the attacker can increase the right side of the previewMintShares by adding rewards for compounding.

That way victim can lose 6e17 of his assets for a deposit of 1e18.

Proof of Concept

Let’s see how a first user, can grief a subsequent deposit and reduce his shares from the desired 1:1 ratio to 1:0000000000000005.

First, he needs to choose PowerFarmToken with no previous deposits.

He calls depositExactAmount with 2 wei which will also call syncSupply → _updateRewards which is a key moment of the attack, this will make it possible PowerFarmController::exchangeRewardsForCompoundingWithIncentive to be called when performing the donation.
With 2 wei user will mint 2 shares, so totalSupply = 3, underlyingLpAssetsCurrent = 3
Then, the victim comes and tries to deposit 1e18 of assets, but is front ran by the first user calling PowerFarmController::exchangeRewardsForCompoundingWithIncentive → addCompoundRewards with 999999999999999996 that will increase the totalLpAssetsToDistribute, which is added to underlyingLpAssetsCurrent in the _syncSupply function, called from the modifier before the main functions.

The attacker does not lose his deposit of 1e18 because it is converted to rewardTokens and sent to him, basically making the inflation free.

PendlePowerFarmController.sol#L53-L111

function exchangeRewardsForCompoundingWithIncentive(
      address _pendleMarket,
      address _rewardToken,
      uint256 _rewardAmount
  ) external syncSupply(_pendleMarket) returns (uint256) {
      CompoundStruct memory childInfo = pendleChildCompoundInfo[_pendleMarket];

      uint256 index = _findIndex(childInfo.rewardTokens, _rewardToken);

      if (childInfo.reservedForCompound[index] < _rewardAmount) {
          revert NotEnoughCompound();
      }

      uint256 sendingAmount = _getAmountToSend(_pendleMarket, _getTokensInETH(_rewardToken, _rewardAmount));

      childInfo.reservedForCompound[index] -= _rewardAmount;
      pendleChildCompoundInfo[_pendleMarket] = childInfo;

      _safeTransferFrom(_pendleMarket, msg.sender, address(this), sendingAmount);

      IPendlePowerFarmToken(pendleChildAddress[_pendleMarket]).addCompoundRewards(sendingAmount);//@audit inflate

      _safeTransfer(childInfo.rewardTokens[index], msg.sender, _rewardAmount);//@audit receive back + incentive

      emit ExchangeRewardsForCompounding(_pendleMarket, _rewardToken, _rewardAmount, sendingAmount);

      return sendingAmount;
  }

After that totalSupply = 3, underlyingLpAssetsCurrent = 1e18 - 1
Victim transaction is executed:

PendlePowerFarmToken.sol#L452-L463

function previewMintShares(uint256 _underlyingAssetAmount, uint256 _underlyingLpAssetsCurrent)
        public
        view
        returns (uint256)
    {
        return _underlyingAssetAmount * totalSupply() / _underlyingLpAssetsCurrent;
        // 1e18 * 3 / 1e18 - 1 = 2 
    }

Both attacker and victim have 1 share, because of the fee that is taken in the deposit function.

After victim deposit:

totalSupply: 5, underlyingLpAssetsCurrent = 2e18 - 1

Then victim tries to withdraw all his shares - 1 (1 was taken as a fee on deposit)

PendlePowerFarmToken.sol#L465-L476

function previewAmountWithdrawShares(uint256 _shares, uint256 _underlyingLpAssetsCurrent)
        public
        view
        returns (uint256)
    {
        return _shares * _underlyingLpAssetsCurrent / totalSupply();
            //1 * (2e18 - 1) / 5 = 399999999999999999 (0.3e18)
    }

User has lost 1e18 - 0.3e18 = 0.6e18 tokens.

Tools Used

Manual Review

Recommended Mitigation Steps

Since there will be many PowerFarmTokens deployed, there is no way team to perform the first deposit for all of them. Possible mitigation will be to have minimum deposit amount for the first depositor in the PendlePowerToken, which will increase the cost of the attack exponentially and there will be no enough of reward tokens making the exchangeRewardsForCompoundingWithIncentive revert, due to insufficient amount.

Or just mint proper amount of tokens in the initialize function.

Assessed type

Other

GalloDaSballo commented 6 months ago

This finding asserts that the rebase changes the price Other 2 reports assert that the rebase will break functionality

c4-pre-sort commented 6 months ago

GalloDaSballo marked the issue as duplicate of #88

c4-pre-sort commented 6 months ago

GalloDaSballo marked the issue as sufficient quality report

GalloDaSballo commented 6 months ago

Seems to show a less impact than primary