c4-bot-10 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PowerFarms/PendlePowerFarmController/PendlePowerFarmToken.sol#L682 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PowerFarms/PendlePowerFarmController/PendlePowerFarmToken.sol#L529 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PowerFarms/PendlePowerFarmController/PendlePowerFarmToken.sol#L452 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PowerFarms/PendlePowerFarmController/PendlePowerFarmToken.sol#L502 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PowerFarms/PendlePowerFarmController/PendlePowerFarmToken.sol#L334 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PowerFarms/PendlePowerFarmController/PendlePowerFarmToken.sol#L386 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PowerFarms/PendlePowerFarmController/PendlePowerFarmToken.sol#L114

Vulnerability details

Vulnerability Details:

The PendlePowerFarmToken contract is initialized through the initialize function, during which the _totalSupply and underlyingLpAssetsCurrent are initially assigned a value of 1.

    function initialize(
        ...
    )
        external
    {
        ...
        lastInteraction = block.timestamp;

        _totalSupply = 1;
        underlyingLpAssetsCurrent = 1;
        mintFee = 3000;
        INITIAL_TIME_STAMP = block.timestamp;
    }

Users can make deposits into the LP through the depositExactAmount function, which relies on the previewMintShares function to calculate the number of shares a user will receive based on the specified _underlyingLpAssetAmount. The calculation of shares is performed using the following formula:

previewMintShares = _underlyingAssetAmount x totalSupply / _underlyingLpAssetsCurrent

Therefore, for the first depositor, given that totalSupply and _underlyingLpAssetsCurrent were initially set to one, it might seem that they would receive a one-to-one ratio. However, this initial assumption could be misleading and may result in the user receiving a less favorable rate.

The contract includes an addCompoundRewards function, allowing any user to add rewards. The amount input through this function is initially added to the totalLpAssetsToDistribute. This totalLpAssetsToDistribute amount is then incorporated into the contract whenever the syncSupply modifier is triggered, which happens during deposits.

Looking closer at the syncSupply mechanism, specifically the _syncSupply() function, it increases the underlyingLpAssetsCurrent with the amount calculated by the previewDistribution function. Although there's a limitation on the increase to underlyingLpAssetsCurrent, a minimum addition of 604799 can be guaranteed, from the following check.

        if (totalLpAssetsToDistribute < ONE_WEEK) {
            return totalLpAssetsToDistribute;
        }

Revisiting the scenario of the first deposit, given the adjustment where the underlyingLpAssetsCurrent has been updated to 604800 and the totalSupply remains at one, let's consider the situation where the first user decides to deposit an amount of 1e18:

previewMintShares = _underlyingAssetAmount x totalSupply / _underlyingLpAssetsCurrent
previewMintShares = 1e18 x 1 / 604800 = 1653439153439

In comparison, a one-to-one ratio would have equated to 1e18 shares, indicating a significant discrepancy from the expected outcome.

Additionally, the lack of an option for users to set slippage limits within the depositExactAmount function makes it challenging to avoid this issue.

NOTE: There's a safeguard in the syncSupply process, specifically through the _validateSharePriceGrowth check, aimed at preventing excessive inflation of the share price. However, because the compound rewards are added prior to the calculation of the initial share price, this is not effective here for the amounts above.

Impact:

Severity: The initial and early depositors are at risk of receiving significantly less favorable rates due to the situation above. Without the capability to set a minimum slippage threshold, this situation could lead to potential losses for these users or discourage them from making deposits altogether.

Likelihood: Since the addCompoundRewards function lacks any form of access control, any user has the capability to execute this attack. Moreover, due to the initial low ratio of 1:1, even a minimal input can lead to significant discrepancies, potentially resulting in substantial losses for early depositors.

Tools Used:

Manual analysis

Recommendation:

The core issue stems from the very low initial values assigned to _totalSupply and underlyingLpAssetsCurrent, both set at one, which leaves the system vulnerable to manipulation. To counteract this, initialize these values at higher numbers.

Assessed type

Other

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as duplicate of #130

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as sufficient quality report