code-423n4 / 2024-02-wise-lending-findings

8 stars 6 forks source link

PendlePowerFarm enterFarm function reverts because of redundant flashLoan call when 1x leverage provided #270

Closed c4-bot-4 closed 3 months ago

c4-bot-4 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/PowerFarms/PendlePowerFarm/PendlePowerManager.sol#L96 https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/PowerFarms/PendlePowerFarm/PendlePowerFarm.sol#L210

Vulnerability details

When entering a farm, a user supplies the _leverage which denotes the amount of leverage to be used while opening the position. In the case the user chooses to enter farm without any leverage, the execution reverts because of the way balance flash loan is executed. This should not happen and there's no need to flash loan any amount if no additional funds are needed as leverage to open the position.

Impact

It prevents users from opening power farm positions when neutral leverage is chosen.

Proof of Concept

Add the snippet to PendlePowerFarmControllerBase.t.sol:

function test_enter_farm_no_leverage() public cheatSetup(false) {
    vm.expectRevert();

    powerFarmManagerInstance.enterFarm(
        false,
        1 ether,
        1 ether,
        entrySpread
    );
}

Run the following command:

forge test --fork-url mainnet --match-test test_enter_farm_no_leverage -vvvv

Tools Used

Manual review

Recommended Mitigation Steps

Execute the _logicOpenPosition directly from PendlePowerFarm without calling the _executeBalancerFlashLoan.

Assessed type

DoS

GalloDaSballo commented 4 months ago

Am not convinced this should be considered even Med, but could be worth looking into

c4-pre-sort commented 4 months ago

GalloDaSballo marked the issue as sufficient quality report

c4-pre-sort commented 4 months ago

GalloDaSballo marked the issue as primary issue

vm06007 commented 3 months ago

Am not convinced this should be considered even Med, but could be worth looking into

let's then put this as a mid for now, but I do agree its not a problem if it reverts, similar as just sanitizing input making sure it is above minimum otherwise revert.

vonMangoldt commented 3 months ago

yea not relevant since 1x leverage here doesnt achieve anything

vm06007 commented 3 months ago

dismissed as invalid, also does not make sense to me entering farm with 1x, should revert as expected

trust1995 commented 3 months ago

Unless 1x leverage is documented as being supported, I fail to see the issue here.

c4-judge commented 3 months ago

trust1995 marked the issue as unsatisfactory: Invalid