pcaversaccio / createx

Factory smart contract to make easier and safer usage of the `CREATE` and `CREATE2` EVM opcodes as well as of `CREATE3`-based (i.e. without an initcode factor) contract creations.
https://createx.rocks
GNU Affero General Public License v3.0
342 stars 26 forks source link

🔒 Prevent Safeguard Mechanisms Bypassing #40

Closed pcaversaccio closed 11 months ago

pcaversaccio commented 11 months ago

🕓 Changelog

This PR introduces a change that for the non-pseudo-random cases, the salt value salt is hashed as part of the _guard function to prevent the safeguard mechanisms from being bypassed. The reason is that if someone deploys on chain A with a preconfigured mechanism, anyone can calculate the inferred guardedSalt and reuse it on chain B. Since we didn't originally modify the salt value in this case, anyone could abuse this. To prevent this, we hash the salt value in those cases (I deliberately don't use inline assembly to have a simple one-liner with a ternary operator which increases readability). The exception is for pseudo-randomly generated salt values, where this is unnecessary. The h/t goes to @lastperson for making me aware of this issue, thanks a lot 🙏!

In the security considerations, the second section still holds true (it would only be frontrun-protected in the case salt == _generateSalt() for the frontrunner on another chain, which would require a hash collision), but the third section is not anymore true (since if you deploy on chain A with a pseudo-random value and someone else uses this on chain B, the value will now be hashed (except for the case if salt == _generateSalt() which is again not likely since you would need to find a hash collision)) and thus I remove it.

This PR is rebased on #37.

🐶 Cute Animal Picture

image

github-actions[bot] commented 11 months ago

Coverage after merging guard-fix into main will be

100.00%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   CreateX.sol100%100%100%100%
mds1 commented 11 months ago

Great catch!