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

TimeswapPair.sol#mint() Lack of input validation allows attacker to set pool.state.y to an arbitrary target value #167

Closed
code423n4 opened this issue Jan 10, 2022 · 1 comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate This issue or pull request already exists

Comments

@code423n4
Copy link
Contributor

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2022-01-timeswap/blob/bf50d2a8bb93a5571f35f96bd74af54d9c92a210/Timeswap/Timeswap-V1-Core/contracts/TimeswapPair.sol#L186-L186

pool.state.y += yIncrease;

TimeswapPair.sol#mint() takes a user input value of yIncrease without proper validation, which means that it allows the state of pool.state.y to increase by the arbitrary value set by the caller.

Impact

  • When pool.state.y is extremely large, many core features of the protocol will malfunction, as the arithmetic related to state.y can overflow. For example:

LendMath.check(): https://github.com/code-423n4/2022-01-timeswap/blob/bf50d2a8bb93a5571f35f96bd74af54d9c92a210/Timeswap/Timeswap-V1-Core/contracts/libraries/LendMath.sol#L28-L28

BorrowMath.check(): https://github.com/code-423n4/2022-01-timeswap/blob/bf50d2a8bb93a5571f35f96bd74af54d9c92a210/Timeswap/Timeswap-V1-Core/contracts/libraries/BorrowMath.sol#L31-L31

  1. An attacker can set state.y to a near overflow value, then lend() to get a large amount of extra interest (as Bond tokens) with a small amount of asset tokens. This way, the attacker can steal funds from other lenders and liquidity providers.

PoC

Near the maturity time, the attacker can do the following:

  1. mint() with a dust amount of assets (xIncrease = 1 wei) and increase pool.state.y to an extremely large value;
  2. lend() a regular amount of assets, get a large amount of bond token;
  3. burn() the bond token and get a large portion of the assets.

Recommendation

Consider making pair.mint() to be onlyConvenience, so that yIncrease will be a computed value (based on xIncrease and current state) rather than a user input value.

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Jan 10, 2022
code423n4 added a commit that referenced this issue Jan 10, 2022
@Mathepreneur Mathepreneur added the duplicate This issue or pull request already exists label Jan 24, 2022
@Mathepreneur
Copy link
Collaborator

Duplicate of #187

@Mathepreneur Mathepreneur marked this as a duplicate of #187 Jan 24, 2022
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 duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

2 participants