code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

When sendNative() is called, cbridge expects _amount of native tokens to be sent to it. However, CBridgeFacet does not send any native asset, resulting in a certain revert.

Proof of Concept

In CBridgeFacet._startBridge, ICBridge(bridge).sendNative() doesn't have value set. Thus no native asset will be sent to the bridge.

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

    function _startBridge(CBridgeData memory _cBridgeData) internal {
        Storage storage s = getStorage();
        address bridge = _bridge();

        // Do CBridge stuff
        require(s.cBridgeChainId != _cBridgeData.dstChainId, "Cannot bridge to the same network.");

        if (LibAsset.isNativeAsset(_cBridgeData.token)) {
            ICBridge(bridge).sendNative(
                _cBridgeData.receiver,
                _cBridgeData.amount,
                _cBridgeData.dstChainId,
                _cBridgeData.nonce,
                _cBridgeData.maxSlippage
            );
        } else {
            // Give CBridge approval to bridge tokens
            LibAsset.approveERC20(IERC20(_cBridgeData.token), bridge, _cBridgeData.amount);
            // solhint-disable check-send-result
            ICBridge(bridge).send(
                _cBridgeData.receiver,
                _cBridgeData.token,
                _cBridgeData.amount,
                _cBridgeData.dstChainId,
                _cBridgeData.nonce,
                _cBridgeData.maxSlippage
            );
        }
    }

Tools Used

Manual code review.

Recommended Mitigation Steps

Set value

            ICBridge(bridge).sendNative{ value: msg.value }(
                _cBridgeData.receiver,
                _cBridgeData.amount,
                _cBridgeData.dstChainId,
                _cBridgeData.nonce,
                _cBridgeData.maxSlippage
            );

H3xept commented 2 years ago

Duplicate of #35

gzeoneth commented 2 years ago

Changing to Med Risk as no fund is lost.

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