Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
code423n4 opened this issue Jul 14, 2023 · 7 comments
Open

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

code423n4 opened this issue Jul 14, 2023 · 7 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working M-08 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue

Comments

@code423n4
Copy link
Contributor

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

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jul 14, 2023
code423n4 added a commit that referenced this issue Jul 14, 2023
@c4-judge
Copy link
Contributor

Picodes marked the issue as primary issue

@asselstine
Copy link

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

@c4-sponsor
Copy link

asselstine marked the issue as sponsor disputed

@c4-sponsor c4-sponsor added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Jul 20, 2023
@Picodes
Copy link

Picodes commented Aug 6, 2023

@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 c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Aug 6, 2023
@c4-judge
Copy link
Contributor

c4-judge commented Aug 6, 2023

Picodes marked the issue as satisfactory

@c4-judge c4-judge added the selected for report This submission will be included/highlighted in the audit report label Aug 6, 2023
@c4-judge
Copy link
Contributor

c4-judge commented Aug 6, 2023

Picodes marked the issue as selected for report

@asselstine
Copy link

Fixed GenerationSoftware/pt-v5-vault#25

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working M-08 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue
Projects
None yet
Development

No branches or pull requests

6 participants