Lack of `_from` address validation in `lzCompose` could lead to loss of user funds

[sujithsomraaj]

Context: ReceiverStargateV2.sol#L84

Description: The function lzCompose is called by LayerzeroEndpoint to deliver a composed message from OApp (stargate in our case); however, the current implementation of lzCompose fails to validate if the _from address is trusted stargate OApp.

As a result, any contract that implements the IPool interface could pose as a valid stargate pool and steal tokens from the ReceiverStargateV2 contract.

Recommendation: Implement checks to ensure the _from address is a valid Stargate OApp address on the relevant chain. For more information, please refer to Layerzero docs and stargate docs.

  function lzCompose(
        address _from,
        bytes32, // _guid (not used)
        bytes calldata _message,
        address, // _executor (not used)
        bytes calldata // _extraData (not used)
    ) external payable onlyEndpointV2 {
+    if(_from != stargateV2) revert();
    }

LI.FI:

Researcher:

[maxklenk]

I fear that is not easily possible as depending from which chain we interact with stargate the address may be different. The main question is, if that is really needed or if stargate itself already validates that. As far as I understood so far it should not matter who the _from address is, because by validating that the calls come from the stargate executor (onlyEndpointV2) we make sure that stargate delivered those tokens to our contract before.

[sujithsomraaj]

Hey @maxklenk, yes, the _from address validation is mandatory. If you take a deeper look into the layerzero endpoint, anyone could compose a message to be delivered to the Receiver contract.

Refer: https://github.com/LayerZero-Labs/LayerZero-v2/blob/417cbb9eb68a4f678490d18728973c8c99f3f017/packages/layerzero-v2/evm/protocol/contracts/MessagingComposer.sol#L23

[sujithsomraaj]

So anyone could be made to call the lzCompose in the receiver contract from the endpoint. So trusting the endpoint alone is not a valid protection

[maxklenk]

I see, thanks for checking. We have to improve that @0xDEnYO

[0xDEnYO]

Yes, that is correct. Very good that you found this one @sujithsomraaj

Since the pool/_from address will differ depending on the token being bridged I would suggest the following mitigation approach:

• add a new address parameter to the payload expStargatePoolAddress and then compare this with _from parameter which leaves it to our backend to determine the correct pool/token and pass it into the payload.

What do you think? @maxklenk @sujithsomraaj

[maxklenk]

As discussed in the internal call today this is not sufficient, but checking that the _from address is a stargate pool by verifying that on the tokenMessenger contract will solve the problem.

[0xDEnYO]

    function lzCompose(
        address _from,
        bytes32, // _guid (not used)
        bytes calldata _message,
        address, // _executor (not used)
        bytes calldata // _extraData (not used)
    ) external payable onlyEndpointV2 {
        // verify that _from address is actually a Stargate pool by checking if Stargate's
        // TokenMessaging contract has an assetId registered for this address
        if (tokenMessaging.assetIds(_from) == 0) revert UnAuthorized();

        // get the address of the token that was received from Stargate bridge
        address bridgedAssetId = IPool(_from).token();

        // decode payload
        (
            bytes32 transactionId,
            LibSwap.SwapData[] memory swapData,
            address receiver
        ) = abi.decode(
                OFTComposeMsgCodec.composeMsg(_message),
                (bytes32, LibSwap.SwapData[], address)
            );

        // execute swap(s)
        _swapAndCompleteBridgeTokens(
            transactionId,
            swapData,
            bridgedAssetId,
            payable(receiver),
            OFTComposeMsgCodec.amountLD(_message)
        );
    }

This is the updated Receiver code that also validates the _from parameter.

Full file: https://github.com/lifinance/contracts/blob/cab07f0be5367fdb6eb93f2994d625ac5f32364d/src/Periphery/ReceiverStargateV2.sol

Please could you comment. Can we close this issue with this solution? @sujithsomraaj @maxklenk @ezynda3

[ezynda3]

Looks good to me. Nice solution.

[sujithsomraaj]

LGTM

[0xDEnYO]

New solution accepted - issue closed.