Boost _ Folks Finance 33695 - [Smart Contract - Critical] Attacker can borrow more than the collateral deposit

Submitted on Fri Jul 26 2024 15:48:54 GMT-0400 (Atlantic Standard Time) by @Shahen for Boost | Folks Finance

Report ID: #33695

Report type: Smart Contract

Report severity: Critical

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

Impacts:

• Protocol insolvency

Description

Brief/Intro

Possibility to borrow more than the deposit amount. Please refer to the below vulnerability details section for the step by step demonstration.

Vulnerability Details

Issue is explained below in a numbered step-by-step way.

1. Attacker makes two 0 amount deposits by calling SpokeCircleToken.deposit(), By making a zero amount deposit will push duplicate pool ids to the list of user loan collaterals because the previous balance is always 0 until a amount greater than 0 is deposited. (L22 UserLoanLogic.sol). So in this situation the pool id will be pushed twice as two zero deposits.

2. Then attacker makes a deposit of 10e6 usdc by calling SpokeCircleToken.deposit()

3. Usually when a user calls SpokeCommon.borrow() LoanManagerLogic.executeBorrow() checks whether the loan is over-collaterised after the borrow by calling userLoan.isLoanOverCollateralized().So this is to prevent from borrowing more then the collateral right. Lets dive into that function. It calls the internal function getLoanLiquidity() which has a for loop in L231 which calculates the effective collateral value. Now since the attacker has pushed the same poolId twice by depositing 0*2 times before the deposit of 10e6, the colPools.length will be 3. Therefore the attacker can borrow from a collateral of 30e6 even though he deposited 10e6 initially.

4. This is considered as stealing of funds as the attacker could borrow more than the capital which could lead to protocol insolvency as debts exceeds the assets. I have attached a foundry coded poc that demonstrates bob the attacker depositing 10e6 and end up borrowing 20e6. Please refer to it. Node rpc is included therefore just RUN forge test -vvv.

Impact Details

Possible to borrow more than the deposit amount.

References

https://github.com/Folks-Finance/folks-finance-xchain-contracts/blob/fb92deccd27359ea4f0cf0bc41394c86448c7abb/contracts/hub/logic/UserLoanLogic.sol#L22

https://github.com/Folks-Finance/folks-finance-xchain-contracts/blob/fb92deccd27359ea4f0cf0bc41394c86448c7abb/contracts/hub/logic/UserLoanLogic.sol#L283

https://github.com/Folks-Finance/folks-finance-xchain-contracts/blob/fb92deccd27359ea4f0cf0bc41394c86448c7abb/contracts/hub/logic/UserLoanLogic.sol#L216

Proof of concept

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "../lib/forge-std/src/Test.sol";


interface iSpokeCommon {
    function createAccount(
        MessageParams memory params,
        bytes32 accountId,
        bytes32 refAccountId
    ) external payable;

    function addDelegate(
        MessageParams memory params,
        bytes32 accountId,
        bytes32 addr
    ) external payable;

    function inviteAddress(
        MessageParams memory params,
        bytes32 accountId,
        uint16 chainId,
        bytes32 addr,
        bytes32 refAccountId
    ) external payable;

    function createLoan(
        MessageParams memory params,
        bytes32 accountId,
        bytes32 loanId,
        uint16 loanTypeId,
        bytes32 loanName
    ) external payable;

    function deleteLoan(
        MessageParams memory params,
        bytes32 accountId,
        bytes32 loanId
    ) external payable;
    
    function withdraw(
        MessageParams memory params,
        bytes32 accountId,
        bytes32 loanId,
        uint8 poolId,
        uint16 chainId,
        uint256 amount,
        bool isFAmount
    ) external payable;

    function borrow(
        MessageParams memory params,
        bytes32 accountId,
        bytes32 loanId,
        uint8 poolId,
        uint16 chainId,
        uint256 amount,
        uint256 maxStableRate
    ) external payable;

}

interface iBridgeRouter {
    function getSendFee(MessageToSend memory message) external view returns (uint256);
}

interface iAccountManager {
     function isAccountCreated(bytes32 accountId) external view returns (bool);
     function isAddressRegisteredToAccount(
        bytes32 accountId,
        uint16 chainId,
        bytes32 addr
    ) external view returns (bool);
    function getAddressRegisteredToAccountOnChain(
        bytes32 accountId,
        uint16 chainId
    ) external view returns (bytes32);
}

interface iHub {
    function directOperation(Action action, bytes32 accountId, bytes memory data) external;
}

interface iLoanManager {
    function isUserLoanActive(bytes32 loanId) external view returns (bool);
    function isUserLoanOwner(bytes32 loanId, bytes32 accountId) external view returns (bool);
    function getUserLoan(
        bytes32 loanId
    )
        external
        view
        returns (
            bytes32 accountId,
            uint16 loanTypeId,
            uint8[] memory colPools,
            uint8[] memory borPools,
            UserLoanCollateral[] memory,
            UserLoanBorrow[] memory
        );


}

interface iHubPool {
    function getPoolId() external view returns (uint8);
}