create2 can be deployed without the msg.sender in salt

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/protocol-contracts/contracts/evm/tools/ImmutableCreate2Factory.sol/#L196

Vulnerability details

Impact

Deployed contracts are hijacked due to accidental msg.sender in salt being 0. Funds sent to these contracts are lost for the users.

Proof of Concept

The containsCaller modifier allows the msg.sender part of the salt to be 0. If user sets this by mistake, then they are suspectible for reorg attacks or cross chain deployment hijacks.

modifier containsCaller(bytes32 salt) {
    // prevent contract submissions from being stolen from tx.pool by requiring
    // that the first 20 bytes of the submitted salt match msg.sender.
    require(
        (address(bytes20(salt)) == msg.sender) || (bytes20(salt) == bytes20(0)),
        "Invalid salt - first 20 bytes of the salt must match calling address."
    );
    _;
}

Attack Scenario:

1. Alice deploys a new contract using the safeCreate2 function, in the next tx she also sends funds to it.
2. Bob, who is monitoring the network, detects a reorganization/deploys it on a different chain and quickly calls the safeCreate2 function with the same salt as Alice's contract.
3. The contract is created for Bob and Alice sends funds to it.
4. Bob can now withdraw the funds sent to the contract, which were originally intended for Alice.

Tools Used

Manual Review

Recommended Mitigation Steps

Don’t allow the address part of the salt to be 0.

Assessed type

Access Control

[c4-pre-sort]

DadeKuma marked the issue as duplicate of #85

[c4-pre-sort]

DadeKuma marked the issue as insufficient quality report

[c4-judge]

0xean marked the issue as unsatisfactory: Insufficient quality