search

code-423n4 / 2023-06-lukso-findings

3 stars 1 forks source link

`CREATE2` address is predictable #52

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/ERC725Alliance/ERC725/blob/31574e1ac5259713e83c1b299800401a0737d483/implementations/contracts/ERC725XCore.sol#L296-L299

Vulnerability details

Impact

Similar to this issue: https://github.com/code-423n4/2023-04-caviar-findings/issues/419

The address of the created contract is predictable and this causes two issues, as mentioned in the above issue:

  1. Stealing deposited amount. If a user creates a new contract and funds it in two separate txs. The first that creates the contract could be frontrun to steal the deposit. Since the contract creation tx would fail but the deposit tx would still succeed.

  2. DoS of creating the contract. A user who creates a contract can be front run by another user who creates the same contract. Thus the first users contract will not be created. This can be repeated denying them to create a contract.

Since we have no idea of what contracts will be deployed by ERC725 any of these could be viable. If any of them sets msg.origin as owner or privileged role this will apply.

Any user abusing this would need the permission to create contracts but this is a pretty non-critical permission so it could be handed out very freely.

Proof of Concept

https://github.com/ERC725Alliance/ERC725/blob/31574e1ac5259713e83c1b299800401a0737d483/implementations/contracts/ERC725XCore.sol#L296-L299

File: implementations/contracts/ERC725XCore.sol

296:        bytes32 salt = BytesLib.toBytes32(
297:            creationCode,
298:            creationCode.length - 32
299:        );

Only publicly reproducible parameters are used in the salt. Hence this can be copied when the tx is in the mem pool.

Tools Used

Manual audit

Recommended Mitigation Steps

Consider adding msg.sender/msg.origin to the salt.

Assessed type

Timing

c4-pre-sort commented 1 year ago

minhquanym marked the issue as primary issue

c4-pre-sort commented 1 year ago

minhquanym marked the issue as low quality report

minhquanym commented 1 year ago

Seems invalid. Only owner can execute actions in this contract

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Invalid