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

Some token pair pools can never be created #37

Closed
code423n4 opened this issue Jan 22, 2023 · 2 comments
Closed

Some token pair pools can never be created #37

code423n4 opened this issue Jan 22, 2023 · 2 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working edited-by-warden unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@code423n4
Copy link
Contributor

code423n4 commented Jan 22, 2023

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-option/src/TimeswapV2OptionDeployer.sol#L32-L35
https://github.com/code-423n4/2023-01-timeswap/blob/ef4c84fb8535aad8abd6b67cc45d994337ec4514/packages/v2-pool/src/TimeswapV2PoolDeployer.sol#L25-L29

Vulnerability details

Impact

when try to deploy pool or option contract if created address has been deployed before, there is no chance to create this pair.

Proof of Concept

Due to creation of pool and option contracts are deterministic and there is no chance to create with different address in optionFactory and deploymentFactory contracts,it may behave unexpected in future because of the generated address has probably been created in ethereum chain before by someone.This situation cause EVM error revert. At that position there is no mechanism to deploy with different address so desired pair pool or option contract can't be created in any way.
This issue discussed in ethereum/EIPs#684.

Tools Used

Recommended Mitigation Steps

Can be given permission to owner for change create mechanism if creating of specific pair is reverted. Another way is use a nonce value.Start with 0 and if contract creation is reverted increase nonce value 1.
There is a exapmle code for using nonce escape mechanism.
https://imgur.com/sTa3Vwd

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Jan 22, 2023
code423n4 added a commit that referenced this issue Jan 22, 2023
@Picodes
Copy link

Picodes commented Jan 29, 2023

Considering the salt this would be a hash collision so we should be safe.

@c4-judge
Copy link
Contributor

Picodes marked the issue as unsatisfactory:
Invalid

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Jan 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working edited-by-warden unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants