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

Griefing attack can prevent almost all activity in a pool #133

Closed
code423n4 opened this issue Jan 10, 2022 · 1 comment
Closed

Griefing attack can prevent almost all activity in a pool #133

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

harleythedog

Vulnerability details

Impact

Consider the mint function in TimeswapPair.sol. The caller of this function is able to freely specify xIncrease, yIncrease and zIncrease. In particular, it is possible to specify xIncrease and zIncrease to be extremely small values (e.g. 1 wei), while yIncrease can be set to be arbitrarily large. In this specific scenario there would be very little cost to the caller (only need to transfer 2 wei of asset/collateral), and the global pool.state.y value would be increased to be a huge value. Due to the large yIncrease value, the caller would have an enormous debt, but since they transferred such a small amount to the contract, they would not care about repaying their position.

The consequences of this are severe. Suppose a malicious user executes such an attack, and spends 2 wei of asset/collateral to increase pool.state.y to equal type(uint112).max. Here are some immediate consequences:

  • No one will be able to borrow. Indeed, someone who calls borrow must have yIncrease >= ymax / 16 > 0, but any increase of pool.state.y will result in an overflow and thus the transaction will revert.
  • No one will be able to mint. Indeed, mint enforces that yIncrease > 0 but any increase of pool.state.y will result in an overflow and thus the transaction will revert.
  • Lenders will need to transfer huge amounts to participate. We know that ymax = Y - K/(Z*(X+xIncrease)), and thus since Y is large, ymax will be large. Since it is enforced that yDecrease >= ymax / 16, lenders will need to have enormous xIncrease values to maintain the constant product.

This means almost all activity on the pool will be halt due to reverts, so users will be forced to withdraw and find a pool with a different maturity date that has not been griefed. Not only is this annoying to the user, but they will also have to incur transaction fees. It is stated in the whitepaper that it is expected that there will likely only be a handful of maturity dates with significant activity (as opposed to thousands of small pools at different maturity dates), so this griefing attack would be quite devastating to these handful of pools.

Proof of Concept

To prove that this is possible, I have created a test script to showcase the exploit here: https://github.com/rholterhus/Code4rena-POCs/tree/main/TimeLock. An example test output from this test case is:

BEFORE ATTACK:
pool.state.x: 3752525435345546043365248164610047
pool.state.y: 447798384943763396004000000000000
pool.state.z 14990373939507790482139864568390
Attacker asset balanceOf: BigNumber { value: "10000" }
Attacker collateral balanceOf: BigNumber { value: "10000" }
AFTER ATTACK:
pool.state.x: 3752525435345546043365248164610048
pool.state.y: 5192296858534827628530496329220095
pool.state.z 14990373939507790482139864568391
Attacker asset balanceOf: BigNumber { value: "9999" }
Attacker collateral balanceOf: BigNumber { value: "9998" }

Notice that the attacker spends 1 asset wei, 2 collateral wei, and increases pool.state.y to equal 2^112 - 1 (the maximum value of pool.state.y). The consequences of such an attack have already been explained above.

NOTE: One of the reasons that such an attack can be so cheap is that mint determines the amount of collateral needed by:

uint256 _collateralIn = maturity;
_collateralIn -=  block.timestamp;
_collateralIn *= zIncrease;
_collateralIn = _collateralIn.shiftRightUp(25);
_collateralIn += zIncrease;
collateralIn = _collateralIn.toUint112();

And if zIncrease = 1 and maturity - block.timestamp < 2^25 - 1 ~ 1.06 years , then the bitshift right will move all digits to zero, so collateralIn will end up just equaling 2.

Tools Used

Hardhat.

Recommended Mitigation Steps

The main issue with this attack is that the griefer is able to spend essentially nothing (2 wei) to maximize the pool.state.y value, ruining the functionality of the contract in the process. I believe that if a minimum LP position was enforced (e.g. mint requires you transfer a minimum amount to the contract), then this griefing attack will not happen as it will be too costly. Another idea of a fix is to cap how large yIncrease can be in the mint function, based on xIncrease and zIncrease.

@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

Dulicate of #187

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