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

6 stars 4 forks source link

CBridgeFacet's startBridgeTokensViaCBridge do not revert on excess msg.value #55

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

Any native funds mistakenly sent along with startBridgeTokensViaCBridge bridging call in excess of _cBridgeData.amount will not be used, will remain frozen on the contract balance and can be lost.

Similarly to the zero check issue placing the severity to be medium as in combination with other issues there is a possibility for user funds to be frozen for an extended period of time (if WithdrawFacet's issue plays out) or even lost (if LibSwap's swap native tokens one also be triggered).

Proof of Concept

startBridgeTokensViaCBridge allows msg.value to be greater than _cBridgeData.amount:

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

The excess isn't needed and only _cBridgeData.amount is subsequently used:

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

Recommended Mitigation Steps

Consider reverting when startBridgeTokensViaCBridge is called with any excess native amount.

Now:

require(msg.value >= _cBridgeData.amount, "ERR_INVALID_AMOUNT");

To be:

require(msg.value == _cBridgeData.amount, "ERR_INVALID_AMOUNT");
H3xept commented 2 years ago

Duplicate of #207