`TimeswapPair.sol#borrow()` Attacker can increase `pool.state.y` to an arbitrary target value

[code423n4]

Handle

WatchPug

Vulnerability details

TimeswapPair.sol#borrow() takes a user input value of yIncrease, and the BorrowMath.check() at L316 only checks for a minimal yIncrease, which means that it allows the state of pool.state.y to increase by the value of the user's input as long as it's larger than minimal.

Even though a large number of yIncrease means that the user will need to pay more interest, the attacker can use a small amount xDecrease (1 wei for example) so that the collateral can be a small amount and simply default the loan at minimal cost (dust amount of collateral).

As a result, pool.state.y can be set to an arbitrary target value by the attacker.

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

function borrow(
    uint256 maturity,
    address assetTo,
    address dueTo,
    uint112 xDecrease,
    uint112 yIncrease,
    uint112 zIncrease,
    bytes calldata data
) external override lock returns (uint256 id, Due memory dueOut) {
    require(block.timestamp < maturity, 'E202');
    require(assetTo != address(0) && dueTo != address(0), 'E201');
    require(assetTo != address(this) && dueTo != address(this), 'E204');
    require(xDecrease > 0, 'E205');

    Pool storage pool = pools[maturity];
    require(pool.state.totalLiquidity > 0, 'E206');

    BorrowMath.check(pool.state, xDecrease, yIncrease, zIncrease, fee);

    dueOut.debt = BorrowMath.getDebt(maturity, xDecrease, yIncrease);
    dueOut.collateral = BorrowMath.getCollateral(maturity, pool.state, xDecrease, zIncrease);
    dueOut.startBlock = BlockNumber.get();

    Callback.borrow(collateral, dueOut.collateral, data);

    id = pool.dues[dueTo].insert(dueOut);

    pool.state.reserves.asset -= xDecrease;
    pool.state.reserves.collateral += dueOut.collateral;
    pool.state.totalDebtCreated += dueOut.debt;

    pool.state.x -= xDecrease;
    pool.state.y += yIncrease;
    pool.state.z += zIncrease;

    asset.safeTransfer(assetTo, xDecrease);

    emit Sync(maturity, pool.state.x, pool.state.y, pool.state.z);
    emit Borrow(maturity, msg.sender, assetTo, dueTo, xDecrease, id, dueOut);
}

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

• 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. borrow() a dust amount of assets (xDecrease = 1 wei) and increase pool.state.y to an extremely large value;
2. lend() a regular amount of state.x, get a large amount of bond token;
3. burn() the bond token and get a large portion of the assets.

Recommendation

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

[Mathepreneur]

Duplicate of #169