code-423n4 / 2023-07-pooltogether-findings

12 stars 7 forks source link

Attacker can frontrun deployVault to deploy at the same address #416

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/VaultFactory.sol#L67-L78

Vulnerability details

Impact

Vaults are created from the factory via CREATE1, an attacker can frontrun deployVault to deploy at the same address but with different config. If the deployed chain reorg, a different vault might also be deployed at the same address.

Proof of Concept

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/VaultFactory.sol#L67-L78

  1. Bob setup a bot to monitor the mempool when PT deploy a new vault
  2. Bob's bot saw a deployment by PT at 0x1234, fire a tx to deposit immediately
  3. Alice frontrun PT's deployment by deploying a malicious vault at 0x1234
  4. Bob's transaction ended up deposited into Alice's malicious vault

Recommended Mitigation Steps

Use CREATE2 and the vault config as salt.

Assessed type

MEV

c4-judge commented 1 year ago

Picodes marked the issue as primary issue

asselstine commented 1 year ago

The Vault address is derivative of the (sender address, nonce). I don't see how this scenario is possible?

c4-sponsor commented 1 year ago

asselstine marked the issue as sponsor disputed

Picodes commented 11 months ago

@asselstine exactly, so here it only depends on the nonce of the factory, so in case of reorg someone could "override" a vault deployment and all following transactions would still be executed

c4-judge commented 11 months ago

Picodes marked the issue as satisfactory

c4-judge commented 11 months ago

Picodes marked the issue as selected for report

asselstine commented 11 months ago

Fixed https://github.com/GenerationSoftware/pt-v5-vault/pull/25