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

Remove salt from createPair() #109

Open
code423n4 opened this issue Jan 8, 2022 · 1 comment
Open

Remove salt from createPair() #109

code423n4 opened this issue Jan 8, 2022 · 1 comment
Labels
bug Something isn't working G (Gas Optimization) sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons

Comments

@code423n4
Copy link
Contributor

Handle

sirhashalot

Vulnerability details

Impact

The createPair() function in TimeswapFactory.sol passes a salt value to when creating a new TimeswapPair. This salt is never used. Most likely the salt is an artifact from Uniswap V2 Core code. However, Uniswap v2 Core only uses a salt for the CREATE2 assembly call, which is not used in the Timeswap project.
https://docs.uniswap.org/protocol/V2/guides/smart-contract-integration/getting-pair-addresses

Proof of Concept

The unnecessary salt is found at:
https://github.com/code-423n4/2022-01-timeswap/blob/bf50d2a8bb93a5571f35f96bd74af54d9c92a210/Timeswap/Timeswap-V1-Core/contracts/TimeswapFactory.sol#L52

Recommended Mitigation Steps

Don't pass a salt when creating a pair. The revised line of code should look like:
pair = new TimeswapPair(asset, collateral, fee, protocolFee);

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Jan 8, 2022
code423n4 added a commit that referenced this issue Jan 8, 2022
@Mathepreneur Mathepreneur added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Jan 18, 2022
@Mathepreneur
Copy link
Collaborator

It is currently not in used, but a different convenience contract or proxy contract might find this feature useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working G (Gas Optimization) sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons
Projects
None yet
Development

No branches or pull requests

2 participants