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.

code-423n4 / 2022-01-timeswap-findings