Caller can lose ETH while bridging

[code423n4]

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/NXTPFacet.sol#L133

Vulnerability details

Impact

A user wanting to bridge ERC20 tokens would lose all ETH accidentally send to the contract.

Proof of Concept

The bridging facets follow the pattern of checking if the token is native or ERC20. In case the token is ERC20, there is no check that msg.value == 0 (except for NXTPFacet::completeBridgeTokensViaNXTP).

This leads to the ETH sent accidentally by a user being held unaccounted for in the contract and being lost for the user.

Recommended Mitigation Steps

Refactor following functions to include a check that msg.value == 0 in case the token address is not zero:

• CBridgeFacet::startBridgeTokensViaCBridge
• CBridgeFacet::swapAndStartBridgeTokenViaCBridge
• AnyswapFacet::startBridgeTokensViaAnyswap
• AnyswapFacet::swapAndStartBridgeTokensViaAnyswap
• NXTPFacet::swapAndStartBridgeTokensViaNXTP
• NXTPFacet::swapAndStartBridgeTokensViaAnyswap
• HopFacet::swapAndStartBridgeTokensViaHop
• HopFacet::startBridgeTokensViaHop

[H3xept]

Duplicate of https://github.com/code-423n4/2022-03-lifinance-findings/issues/53