Timeswap contest - ye0lde's results

Handle

ye0lde

Vulnerability details

Impact

The function below fails to perform input validation on arrays to verify the lengths match. A mismatch could lead to an exception or undefined behavior.

Proof of Concept

ids, assetsIn (copied into from maxAssetsIn on line 18) https://github.com/code-423n4/2022-01-timeswap/blob/bf50d2a8bb93a5571f35f96bd74af54d9c92a210/Timeswap/Timeswap-V1-Convenience/contracts/libraries/PayMath.sol#L15-L28

While givenMaxAssetsIn is an internal function if you trace the code back the parameters are passed in by an external function (pay or payEthAsset or payEthCollateral) with no array length validation.

Tools Used

Visual Studio Code, Remix

Recommended Mitigation Steps

Add input validation to check that the length of all arrays match (ids, maxAssetsIn).

Handle

ye0lde

Vulnerability details

Impact

The pool.state.totalLiquidity is increased instead of set to the value directly. This performs an unnecessary addition as pool.state.totalLiquidity is zero.

Proof of Concept

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

Tools Used

Visual Studio Code, Remix

Recommended Mitigation Steps

Change

pool.state.totalLiquidity += liquidityTotal;

to

pool.state.totalLiquidity = liquidityTotal;

Handle

ye0lde

Vulnerability details

Impact

Gas savings

Proof of Concept

"> 0" is used in the following locations: https://github.com/code-423n4/2022-01-timeswap/blob/bf50d2a8bb93a5571f35f96bd74af54d9c92a210/Timeswap/Timeswap-V1-Convenience/contracts/libraries/SquareRoot.sol#L21 https://github.com/code-423n4/2022-01-timeswap/blob/bf50d2a8bb93a5571f35f96bd74af54d9c92a210/Timeswap/Timeswap-V1-Convenience/contracts/libraries/PayMath.sol#L27

Tools Used

Visual Studio Code, Remix

Recommended Mitigation Steps

Change "> 0" to "!=0" for small gas savings.

Handle

ye0lde

Vulnerability details

Impact

Redundant arithmetic underflow/overflow checks can be avoided when an underflow/overflow cannot happen.

Proof of Concept

The "unchecked" keyword can be applied here since there is an "if" statement to ensure the arithmetic operations would not cause an integer underflow or overflow. https://github.com/code-423n4/2022-01-timeswap/blob/bf50d2a8bb93a5571f35f96bd74af54d9c92a210/Timeswap/Timeswap-V1-Convenience/contracts/libraries/Mint.sol#L278

Change the code at 278 to:

        unchecked { 
		 if (maxCollateral > dueOut.collateral) ETH.transfer(payable(msg.sender), maxCollateral - dueOut.collateral);
	} 

Similar changes can be made here: https://github.com/code-423n4/2022-01-timeswap/blob/bf50d2a8bb93a5571f35f96bd74af54d9c92a210/Timeswap/Timeswap-V1-Convenience/contracts/libraries/Mint.sol#L396 https://github.com/code-423n4/2022-01-timeswap/blob/bf50d2a8bb93a5571f35f96bd74af54d9c92a210/Timeswap/Timeswap-V1-Convenience/contracts/libraries/Mint.sol#L436 https://github.com/code-423n4/2022-01-timeswap/blob/bf50d2a8bb93a5571f35f96bd74af54d9c92a210/Timeswap/Timeswap-V1-Convenience/contracts/libraries/Mint.sol#L557

https://github.com/code-423n4/2022-01-timeswap/blob/bf50d2a8bb93a5571f35f96bd74af54d9c92a210/Timeswap/Timeswap-V1-Convenience/contracts/libraries/Pay.sol#L62

Tools Used

Visual Studio Code, Remix

Recommended Mitigation Steps

Add the "unchecked" keyword as shown above.