code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L76 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/CBridgeFacet.sol#L94 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/GenericSwapFacet.sol#L22 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/HopFacet.sol#L97 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/NXTPFacet.sol#L87 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/NXTPFacet.sol#L152

Vulnerability details

Description

As per the documentation, most of the contracts using swapData as calldata input have an input field deadline and destinationDeadline The purpose of them as per the documentation, is "The time the transaction must be completed or revert." Ref:- https://github.com/code-423n4/2022-03-lifinance/blob/main/docs/HopFacet.md

However, it was noticed that none of the contracts validate the time of the deadline, and the logic to revert a transaction after a deadline will never work.

Impact

Since the deadline and the destinationDeadline are never validated in any of the contracts, the transaction will take forever and will never revert in the given time period.

Proof of Concept

We will notice that swapData has an input called deadline and destinationDeadline
Below is the list of contracts and functions that use swapData

a) swapAndStartBridgeTokensViaAnyswap https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L76 b) swapAndStartBridgeTokensViaCBridge https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/CBridgeFacet.sol#L94 c) swapTokensGeneric https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/GenericSwapFacet.sol#L22 d) swapAndStartBridgeTokensViaHop https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/HopFacet.sol#L97 e) swapAndStartBridgeTokensViaNXTP https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/NXTPFacet.sol#L87 f) swapAndCompleteBridgeTokensViaNXTP https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/NXTPFacet.sol#L152

We will notice none of the functions add any validation for deadline & destinationDeadline which can even be in past. And the transaction will still go through.

For PoC, taking example of CBridge: Pass the below value where (deadline = Math.floor(Date.now() / 1000) - 60 * 20) - This will create a deadline in past.

    const swapData = await uniswap.populateTransaction.exactOutputSingle([
      USDC_ADDRESS,
      DAI_ADDRESS,
      3000,
      to,
      deadline,
      amountOut,
      amountIn,
      0,
    ])

We will notice the transaction did not fail as there is no validation from the smart contract regarding the deadlines.

Recommended Mitigation Steps

Add a validation check for the deadline and destinationDeadline to validate the swap transactions.

maxklenk commented 2 years ago

The deadlines are not part of our custom parameters, instead we always forward them to the Bridge/DEX we are using and where they will be validated.

e.g. in Uniswap: https://github.com/Uniswap/v2-periphery/blob/master/contracts/UniswapV2Router02.sol#L19

gzeoneth commented 2 years ago

Invalid submission.

code-423n4 / 2022-03-lifinance-findings