Boost _ Folks Finance 34047 - [Smart Contract - Low] Adversaries can create a position that is nearl

Submitted on Sun Aug 04 2024 16:20:03 GMT-0400 (Atlantic Standard Time) by @nnez for Boost | Folks Finance

Report ID: #34047

Report type: Smart Contract

Report severity: Low

Target: https://testnet.snowtrace.io/address/0xf8E94c5Da5f5F23b39399F6679b2eAb29FE3071e

Impacts:

• Protocol insolvency

Description

Description

Users' loan position is stored in a mapping of a struct UserLoan

struct UserLoan {
    bool isActive;
    bytes32 accountId;
    uint16 loanTypeId;
    uint8[] colPools;
    uint8[] borPools;
    mapping(uint8 poolId => UserLoanCollateral) collaterals;
    mapping(uint8 poolId => UserLoanBorrow) borrows;
}
mapping(bytes32 loanId => UserLoan) internal _userLoans;

The borPools member keeps a list of poolId values representing the pools that users have positions in. This is similar to how other lending platforms track the markets a user has entered.

Below is the code snippet responsible for adding a new poolId to user's loan.

function increaseBorrow(
    LoanManagerState.UserLoan storage loan,
    DataTypes.UpdateUserLoanBorrowParams memory params,
    bool isStable
) external {
    LoanManagerState.UserLoanBorrow storage loanBorrow = loan.borrows[params.poolId];

    if (loanBorrow.balance == 0) {
        initLoanBorrowInterests(loanBorrow, params, isStable);
        loanBorrow.amount = params.amount;
        loanBorrow.balance = params.amount;
        loan.borPools.push(params.poolId); @<--
    } else {
        if (isStable != loanBorrow.stableInterestRate > 0) revert BorrowTypeMismatch();
        updateLoanBorrowInterests(loanBorrow, params);

        // update amount
        loanBorrow.amount += params.amount;
        loanBorrow.balance += params.amount;
    }
}

If the current borrow balance for the poolId is zero, the system adds the poolId to borPools. However, this approach has a flaw: it's possible to borrow an amount of 0. If the borrow function is called with an amount of 0 multiple times, the increaseBorrow function will repeatedly add the same poolId to borPools.

borPools is used in getLoanLiquidity to calculate the effective value of the loan.

function getLoanLiquidity(
    LoanManagerState.UserLoan storage loan,
    mapping(uint8 => IHubPool) storage pools,
    mapping(uint8 => LoanManagerState.LoanPool) storage loanPools,
    IOracleManager oracleManager
) internal view returns (DataTypes.LoanLiquidityParams memory loanLiquidity)
    ...
    ... snipped
    ...
    poolsLength = loan.borPools.length;
    for (uint8 i = 0; i < poolsLength; i++) { @<-- maximum iteration = 255
        poolId = loan.borPools[i];

        LoanManagerState.UserLoanBorrow memory loanBorrow = loan.borrows[poolId];
        balance = loanBorrow.lastStableUpdateTimestamp > 0
            ? calcStableBorrowBalance(
                loanBorrow.balance,
                loanBorrow.lastInterestIndex,
                loanBorrow.stableInterestRate,
                block.timestamp - loanBorrow.lastStableUpdateTimestamp
            )
            : calcVariableBorrowBalance(
                loanBorrow.balance,
                loanBorrow.lastInterestIndex,
                pools[poolId].getUpdatedVariableBorrowInterestIndex()
            );
        priceFeed = oracleManager.processPriceFeed(poolId);
        effectiveValue += MathUtils.calcBorrowAssetLoanValue(
            balance,
            priceFeed.price,
            priceFeed.decimals,
            loanPools[poolId].borrowFactor
        );
    }
    loanLiquidity.effectiveBorrowValue = effectiveValue;
    ...
    ... snipped

We can see that it iterates over the entire length of the array. Each iteration incurs a significant gas cost because it needs to retrieve the price from the oracle during every iteration. An adversary could exploit this by adding multiple poolId values to the array (up to a maximum of 255). This would increase the gas cost significantly, potentially even reaching the block gas limit, making it impossible to liquidate their position as decribe in the following attack scenario.

Attack Scenario

1. Attacker deposits AVAX as collateral

2. Attacker borrows USDC out

Attacker's borPools = [USDC]

1. Attacker starts repeatedly borrowing AVAX with amount=0

Attacker's borPool = [USDC, AVAX, AVAX, AVAX, ...]

Each iteration increases the gas consumption to liquidate attacker's position because it increases the number of iterations in UserLoanLogic#getLoanLiquidity which is called in liquidation execution flow.

1. Attacker can keep repeating Step 3 to the maximum of 255 iterations.

In cases where a complex oracle is used (such as nodes with many parents), the gas consumption for liquidation could reach the block gas limit, making it impossible to liquidate the attacker's position.

While the attacker does have to cover the gas fees to create this position, the cost is minimal. Even in the final iteration, where the gas usage approaches the block limit, the expense remains under $20.

Impact

• This could result in bad debt for the protocol, as the attacker's position cannot be liquidated.

Recommended Mitigations

• Consider setting a maximum limit on the borrow and collateral pools for a single loan.

Proof of concept

Proof-of-Concept

A test in the secret gist demonstrates the attack scenario described. It shows that an attacker with a borrow pool length of 255 could potentially incur a cost of 13 million gas units to execute the liquidation.

Steps

1. Run forge init --no-commit --no-git --vscode.

2. Create a new test file, FolksNotLiquidate.t.sol in test directory.

3. Put the test from secret gist in the file: https://gist.github.com/nnez/6b8c828a6d38b202525bf3c98d532a11

4. Run forge t --match-contract FolksNotAbleToLiquidateTest -vv

5. Observe the gas used to perform liquidation on ALICE's position.