CREATE2 address collision during pool deployment allows for complete draining of the pool before the assertion is created on the Rollup contract

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/main/src/assertionStakingPool/EdgeStakingPoolCreator.sol#L19
https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/main/src/assertionStakingPool/AssertionStakingPoolCreator.sol#L19

Vulnerability details

Impact

Funds transferred to an Assertion/Edge StakingPool can be drained by an attacker via a hash collision.

Proof of Concept

(NOTE: This report is inspired from this past valid report. Necessary changes have been made to suit the Arbitrum BoLD Protocol.)

The attack consists of two parts: Finding a collision and actually draining the lending pool. We describe both here:

PoC: Finding a collision
The pools created using the AssertionStakingPoolCreator::createPool() function or the EdgeStakingPoolCreator::createPool() function sets the salt as 0, which means, the final address where the contract will be deployed relies solely on the parameters of the constructor of the newly created contract, in this case, the rollup and assertionHash.

function createPool(
    address _rollup,
    bytes32 _assertionHash
) external returns (IAssertionStakingPool) {
    //@audit-issue => Setting the salt as 0 causes the final address to be determined only by the two parameters passed to the constructor of the new contract
    AssertionStakingPool assertionPool = new AssertionStakingPool{salt: 0}(_rollup, _assertionHash);
    ...
}

PoC: Assertion/Edge StakinPoolCreator contracts deploy the contracts to a deterministic address
Add the below test to the AssertionStakingPool.t.sol test file

• Run it and notice that the address where the contract is tried to be deployed to it is the same as the address of the first time the assertionHash was deployed to (during the setup())
  • Because of the salt set as 0, the nonce of the number of deployed contracts doesn't matter.

function test_CreatePoolPoC() external {
    //@audit-info => The PoolCreator contract given the same parameters deploys the pool to the same address.
        //@audit => The address of the newly created pool would always be the same given the same parameters
    vm.expectRevert();
    IAssertionStakingPool newPool = aspcreator.createPool(address(rollupAddr), assertionHash);
}

The address collision an attacker will need to find are:

• The address where an assertionHash would be deployed to (1).
• Arbitrary attacker-controlled wallet contract (2).

Both sets of addresses can be brute-force searched because:

• As shown above, salt is set as 0, thus, the final address is determined only by the two parmeters passed to the constructor. By brute-forcing many assertionHash values, we have obtained many different (undeployed) wallet accounts for (1). The user can know the address of the Edge/Assertion StakingPool before deploying it, since as shown in the above code snippet, the result is deterministic.
• (2) can be searched the same way. The contract just has to be deployed using CREATE2, and the salt is in the attacker's control by definition.

An attacker can find any single address collision between (1) and (2) with high probability of success using the following meet-in-the-middle technique, a classic brute-force-based attack in cryptography:

• Brute-force a sufficient number of values of assertionHash (2^80), pre-compute the resulting account addresses, and efficiently store them e.g. in a Bloom filter data structure.
• Brute-force contract pre-computation to find a collision with any address within the stored set in step 1.

The feasibility, as well as detailed technique and hardware requirements of finding a collision, are sufficiently described in multiple references:

• 1: A past issue on Sherlock describing this attack.
• 2: EIP-3607, which rationale is this exact attack. The EIP is in final state.
• 3: A blog post discussing the cost (money and time) of this exact attack.

The hashrate of the BTC network has reached 6.5x10^20 hashes per second as of time of writing, taking only just 31 minutes to achieve 2^80 hashes. A fraction of this computing power will still easily find a collision in a reasonably short timeline.

PoC: Draining the lending pool
Even given EIP-3607 which disables an EOA if a contract is already deployed on top, we show that it's still possible to drain the Assertion/Edge StakingPool entirely given a contract collision.

Assuming the attacker has already found an address collision against an undeployed Assertion/Edge StakingPool, let's say 0xCOLLIDED. The steps for complete draining of the Assertion/Edge StakingPool are as follow:

First tx:

• Deploy the attack contract onto address 0xCOLLIDED.
• Set infinite allowance for {0xCOLLIDED ---> attacker wallet} for any token they want.
• Destroy the contract using selfdestruct.

Post Dencun hardfork, selfdestruct is still possible if the contract was created in the same transaction. The only catch is that all 3 of these steps must be done in one tx.

The attacker now has complete control of any funds sent to 0xCOLLIDED.

Second tx:

• Deploy the Assertion/Edge StakingPool to 0xCOLLIDED.
• Wait until the Assertion/Edge StakingPool will hold as many tokens as you want and drain it.

The attacker has stolen all funds from the Assertion/Edge StakingPool.

Proof of Concept

While we cannot provide an actual hash collision due to infrastructural constraints, we are able to provide a coded PoC to prove the following two properties of the EVM that would enable this attack:

• A contract can be deployed on top of an address that already had a contract before.
• By deploying a contract and self-destruct in the same tx, we are able to set allowance for an address that has no bytecode.

Here is the PoC, as well as detailed steps to recreate it:

• Paste the following file onto Remix (or a developing environment of choice): POC
• Deploy the contract Test.
• Run the function Test.test() with a salt of your choice, and record the returned address. The result will be:
  • Test.getAllowance() for that address will return exactly APPROVE_AMOUNT.
  • Test.getCodeSize() for that address will return exactly zero.
  • This proves the second property.
• Using the same salt in step 3, run Test.test() again. The tx will go through, and the result will be:
  • Test.test() returns the same address as with the first run.
  • Test.getAllowance() for that address will return twice of APPROVE_AMOUNT.
  • Test.getCodeSize() for that address will still return zero.
  • This proves the first property.

Tools Used

Manual Audit

Recommended Mitigation Steps

Don't use an empty salt to create the pools.

• An empty salt makes that the address where the pool is deployed to be completely deterministic, the final address relies solely on the parameters of the constructor, in this case, the rollup and the assertionHash. This allows attackers to play with the assertionHash parameters to finds collisions so that they can perform the attack explained on this report.
• Consider generating a hash encoding the block.timestamp and block.number. This would make impossible for an attacker to compute collisions in advance, given that now the address of the deployed contract would depend on the exact block.timestamp and block.number when the contract is created.
  • This new implementation would require to update the logic to keep track of the created pools. For this, I'd recommend to track the created pool via a mapping, and, each time a new pool is created, associate the parameters that were used to create the new pool with the resultant address of the newly created pool

+ mapping (address => mapping (bytes32 => address)) deployedPools;

function createPool(
    address _rollup,
    bytes32 _assertionHash
) external returns (IAssertionStakingPool) {
-   AssertionStakingPool assertionPool = new AssertionStakingPool{salt: 0}(_rollup, _assertionHash);
+   bytes32 _salt = keccak256(abi.encodePacked(block.timestamp, block.number));
+   AssertionStakingPool assertionPool = new AssertionStakingPool{salt: _salt}(_rollup, _assertionHash);
+   deployedPools[_rollup][_assertionHash] = address(assertionPool);
    emit NewAssertionPoolCreated(_rollup, _assertionHash, address(assertionPool));
    return assertionPool;
}

function getPool(
    address _rollup,
    bytes32 _assertionHash
) public view returns (IAssertionStakingPool) {
    return IAssertionStakingPool(deployedPools[_rollup][_assertionHash]);
}

Assessed type

Other

[stalinMacias]

hello judge @Picodes

I'd like to bring this report to your attention since it looks like it didn't pass the Validation's queue.

This report is addressing the exact same issue about address collission that was assesed as a valid medium in the Panoptic Contest hosted in April 2024

Though, in this case, the funds at stake are greather, an AssertionPool would needs to collect the same amount of funds that a single validator would need to, ~1k ETH.

I'd like to ask if you could take a look at this report and let me know your thoughts about it.

[xuwinnie]

The address collision an attacker will need to find are:

One undeployed Panoptic Pool address (1).
Arbitrary attacker-controlled wallet contract (2).

Hey, it seems to reduce the hash needed to 2^81, a precondition is we can both brutal force (1) and (2). But in the context of this contest, when a staking pool needs to be created, the pool address is uniquely determined. So we will still need 2^160 hashes? Please correct me if I misunderstood anything.

[xuwinnie]

We cannot brutal assertionHash since those randomly generated pools are just meaningless and no ppl would deposit into them.

[stalinMacias]

Hey @xuwinnie

The two pools are created deterministically, assertionPools are created deterministically solely on the parameters passed to the constructor, (which one of the two parameters is the rollup, thus, is a known parameter in advanced), panopticPools are created deterministically based on the salt specified by the user.

You are right about people not depositing into those random generated pools, the attack would be performed on an assertionPool for an assertionHash that would be posted on the Rollup, an assertionHash that needs to crowdfund the required resources for it to be posted.

The setup between the assertionPool and a panopticPool is 95% the same, the two are created deterministically, that's why I wanted to ask @Picodes to know his thoughts on this report, since the same problem was accepted in a recent contest.

[xuwinnie]

It seems in the panoptic contest those pools are not meaningless? (They are just normal pools with correct token and a brutal forced salt)
And honest ppl will only deposit into correct pool with correct assertionHash

[stalinMacias]

And honest ppl will only deposit into correct pool with correct assertionHash

Yes, that's the point of the collision address. Instead of brute forcing a salt as in a PanopticPool, for an AssertionPool is brute forced the assertionHash. That's the only difference between the 2 pools

[gzeoneth]

We cannot brutal assertionHash since those randomly generated pools are just meaningless and no ppl would deposit into them.

Given:

assertionPool = new AssertionStakingPool{salt: 0}(_rollup, _assertionHash);

If the _assertionHash is fixed, you only control _rollup and I don't think a hash collision is realistic.

[Picodes]

The difference between brute forcing a salt in Panoptic and an assertionHash is critical as, as shown by @xuwinnie, no one would deposit in a randomly generated assertionHash, so there is no issue here

[stalinMacias]

@Picodes I just wanted to clarify that the brute forcing would be the same as described on the Panoptic's report, but here, instead of brute forcing the salt that would be used to create the pool, it would be brute forced the assertionHash of an assertionPool where users would deposit to post such assertionHash on the Rollup contract.

In the end, I agree with your judging, I just wanted to comment the exact similute between this report and the one from the Panoptic contest.