Skip to content
This repository has been archived by the owner on Aug 18, 2024. It is now read-only.

PUSH0 - CREATE2 address collision against an Account will allow complete draining of lending pools #59

Open
sherlock-admin2 opened this issue Feb 16, 2024 · 11 comments
Labels
Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin2
Copy link

sherlock-admin2 commented Feb 16, 2024

PUSH0

high

CREATE2 address collision against an Account will allow complete draining of lending pools

Summary

The factory function createAccount() creates a new account contract for the user using CREATE2. We show that a meet-in-the-middle attack at finding an address collision against an undeployed account is possible. Furthermore, such an attack allows draining of all funds from the lending pool.

Vulnerability Detail

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

PoC: Finding a collision

Note that in createAccount, CREATE2 salt is user-supplied, and tx.origin is technically also user-supplied:

account = address(
    new Proxy{ salt: keccak256(abi.encodePacked(salt, tx.origin)) }(
        versionInformation[accountVersion].implementation
    )
);

The address collision an attacker will need to find are:

  • One undeployed Arcadia account address (1).
  • Arbitrary attacker-controlled wallet contract (2).

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

  • As shown above, salt is a user-supplied parameter. By brute-forcing many salt values, we have obtained many different (undeployed) wallet accounts for (1).
  • (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 salt ($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 \times 10^{20}$ hashes per second as of time of writing, taking only just $33$ 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 lending pool entirely given a contract collision.

Assuming the attacker has already found an address collision against an undeployed account, let's say 0xCOLLIDED. The steps for complete draining of a lending pool are as follow:

First tx:

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

Second tx:

  • Deploy an account to 0xCOLLIDED.
  • Deposit an asset, collateralize it, then drain the collateral using the allowance set in tx1.
  • Repeat step 2 for as long as they need to (i.e. collateralize the same asset multiple times).
    • The account at 0xCOLLIDED is now infinitely collateralized.
    • Funds for step 2 and 3 can be obtained through external flash loan. Simply return the funds when this step is finished.
  • An infinitely collateralized account has infinite borrow power. Simply borrow all the funds from the lending pool and run away with it, leaving an infinity collateral account that actually holds no funds.

The attacker has stolen all funds from the lending pool.

Coded unit-PoC

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:

  1. Paste the following file onto Remix (or a developing environment of choice): https://gist.github.com/midori-fuse/087aa3248da114a0712757348fcce814
  2. Deploy the contract Test.
  3. 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.
  4. 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.

The provided PoC has been tested on Remix IDE, on the Remix VM - Mainnet fork environment, as well as testing locally on the Holesky testnet fork, which as of time of writing, has been upgraded with the Dencun hardfork.

Impact

Complete draining of a lending pool if an address collision is found.

With the advancement of computing hardware, the cost of an attack has been shown to be just a few million dollars, and that the current Bitcoin network hashrate allows about $2^{80}$ in about half an hour. The cost of the attack may be offsetted with longer brute force time.

For a DeFi lending pool, it is normal for a pool TVL to reach tens or hundreds of millions in USD value (top protocols' TVL are well above the billions). It is then easy to show that such an attack is massively profitable.

Code Snippet

https://github.com/sherlock-audit/2023-12-arcadia/blob/main/accounts-v2/src/Factory.sol#L96-L100

Tool used

Manual Review, Remix IDE

Recommendation

The mitigation method is to prevent controlling over the deployed account address (or at least severely limit that). Some techniques may be:

  • Do not allow a user-supplied salt, as well as do not use the user address as a determining factor for the salt.
  • Use the vanilla contract creation with CREATE, as opposed to CREATE2. The contract's address is determined by msg.sender (the factory), and the internal nonce of the factory (for a contract, this is just "how many other contracts it has deployed" plus one).

This will prevent brute-forcing of one side of the collision, disabling the $O(2^{81})$ search technique.

@github-actions github-actions bot added the Medium A valid Medium severity issue label Feb 21, 2024
@sherlock-admin2
Copy link
Author

1 comment(s) were left on this issue during the judging contest.

takarez commented:

valid: high(5)

@nevillehuang
Copy link
Collaborator

Request PoC to facilitate discussion between sponsor and watson.

I believe this is low severity given the extreme unlikeliness of it occuring, with the addition that a insanely huge amount of funds is required to perform this attack without guarantee

@sherlock-admin
Copy link
Contributor

PoC requested from @push0

Requests remaining: 8

@midori-fuse
Copy link

midori-fuse commented Feb 23, 2024

Hello, thanks for asking.

The reason we believe this is a valid HM is the following:

  • The impact is undoubtedly high because a pool can be drained completely.
    • Although only one pool can be drained per collision found, that itself is high impact. Furthermore the protocol will be deployed on several EVM chains, so the attacker can simultaneously drain one pool per chain.
  • We are re-citing a past issue on Sherlock discussing the same root cause of CREATE2 collision, as the issue discussion sufficiently describes:
    • The attack cost at the time of the Kyber contest.
    • The probability of a successful attack was shown to increase to 86% using $2^{81}$ hashes, and 99.96% using twice that power. This is not a low probability by any chance.
      • Furthermore one may also find multiple collisions using this technique, which will allow draining of more than one pool.
  • Hardware advancements can only increase the computing power, not decrease it. Since the Kyber contest, BTC hashrate has already increased sigfinicantly (almost doubled), and it has been just 6 months since.
  • The more protocols with this issue there are, the more profit there is to finding a collision.

Therefore the likelihood of this attack can only increase as time passes. Complete draining of a pool also cannot be low severity.

Essentially by identifying this attack, we have proven the existence of a time bomb, that will allow complete draining of a certain pool from any given chain. We understand that past decisions are not considered a source of truth, however the issue discussion and the external resources should still provide an objective reference to determine this issue's validity. Furthermore let's consider the impact if it were to happen as well.

@nevillehuang
Copy link
Collaborator

@Czar102 I am interested in hearing your opinion here with regards to the previous issue here as well.

@Thomas-Smets
Copy link

Don't think it is a high, requires a very big upfront cost with no certainty on pay out as attacker.
And while our factory is immutable, we can pause indefinitely the creation of new accounts.

If at some point the attack becomes a possibility, we can still block it.

To make it even harder, we are thinking to add the block.timestamp and block.number to the hash.
Then the attacker, after they successfully found a hash collision, already has to execute the attack at a fixed block and probably conspire with the sequencer to ensure that also the time is fixed.

@midori-fuse
Copy link

midori-fuse commented Feb 27, 2024

Agree that it's not a high, the attack cost makes a strong external condition.

Re: mitigations. I don't think pausing the contract or blocking the attack makes sense, the attack would sort of finish in a flash before you know it (and in a shorter duration than a pausing multisig can react).

Regarding the fix, adding block.timestamp and block.number technically works, but doesn't really make sense as opposed to just using the vanilla creation. The main purpose of CREATE2 is that contract addresses are deterministic and pre-computable, you can do a handful of things with this information e.g. funds can be sent there in advance before contract creation, or crafting a customized address without deploying. By adding block number and timestamp, the purpose is essentially defeated.

But since it technically works, there's not really a point in opposing it I suppose. A full fix would involve reworking the account's accounting, which I think is quite complex and may open up more problems.

A fix that would still (partially, but should be sufficient) retain the mentioned functionalities could be just using a uint64 salt, or the last 64 bits of the address only, or anything that doesn't let the user determine more than 64 bits of the input. Then the other side of the brute force has to achieve $2^{15} = 32768$ times the mentioned hashing power in the attack to achieve a sizeable collision probability (and such probability is still less than a balanced $2^{80}$ brute force anyway).

@sherlock-admin2 sherlock-admin2 added Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels Feb 27, 2024
@Thomas-Smets
Copy link

I don't think pausing the contract or blocking the attack makes sense

Meant it in the sense that if such an attack occurs we can pause it, assuming we would not be the first victim.
Not that we can frontrun a particular attack with a pause.

Regarding the fix, adding block.timestamp and block.number technically works, but doesn't really make sense as opposed to just using the vanilla creation.

Fair point.

A fix that would still (partially, but should be sufficient) retain the mentioned functionalities could be just using a uint64 salt, or the last 64 bits of the address only, or anything that doesn't let the user determine more than 64 bits of the input.

I do like this idea thanks! We can use 32 bits from the tx.origin and 32 bits from a salt.

@sherlock-admin
Copy link
Contributor

The protocol team fixed this issue in PR/commit arcadia-finance/accounts-v2#176.

@sherlock-admin sherlock-admin changed the title Zealous Alabaster Fly - CREATE2 address collision against an Account will allow complete draining of lending pools PUSH0 - CREATE2 address collision against an Account will allow complete draining of lending pools Feb 28, 2024
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Feb 28, 2024
@IAm0x52
Copy link
Collaborator

IAm0x52 commented Mar 13, 2024

Fix looks good. By using a 32 bit salt the collision is increased from 2^81 to 2^128 dramatically increasing the cost.

@sherlock-admin4
Copy link

The Lead Senior Watson signed off on the fix.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

7 participants