Lack of access control on ``submitMessage`` can allow users to mint tokens for free

[hats-bug-reporter[bot]]

Github username: @PlamenTSV Twitter username: @p_tsanev Submission hash (on-chain): 0x025494d2d02686cd884d7d2d8d599c3e8c1063e95d3b18b5f86f949ec6e32883 Severity: high

Description: Description\ The submitMessage function of the IncentivizedMessageEscrow is usually used by the Catalyst vaults when sending assets cross-chain as the last step of the call chain. The function, apart from taking the payment for the submission and setting the relayer bounty, will always handle the signing of the message so that it's signature can be verified when getting processed. All of the parameters that the function takes are arbitrary and there is no access control, nor input validation at this last step, which can lead to a scenario where a user can get any arbitrary message signed and ready to be processed.

Attack Scenario\ The scenario is quite similiar to 1 of my invalid issues about the signature and 1 issue about a short-term DoS, but it serves as a combination, which does not go away when implementing the fixes proposed by the dev team:

1. A user does submitMessage directly. All of the function parameters are arbitrary and can be crafted, e.g the destination identifier, destination and most importantly - the malicious message itself.
2. The message from step 1 would indefinitely get signed (assumption made per the tests, confirmed by the devs), thus attempting to processPacket it would not revert, since the signature this time would be valid and real - the message really got signed, since this happens in submitMessage.
3. From here on out, the route is similiar: _handleMessage, where if the message is unique we would not revert due to colliding identifiers and if we use a real source implementation, then if (expectedSourceImplementationHash != keccak256(sourceImplementationIdentifier)) would not revert either (we can see real sources from past transaction if the situation calls for it)
4. We go to ICrossChainReceiver(toApplication).receiveMessage, where the only validation is that the provided address is of the 65-byte format, so we can easily pass this by parameter crafting.
5. _handleReceiveLiquidity into ICatalystV1Vault(toVault).receiveLiquidity, which on both the Volatile and Amplified vaults would mint vault tokens, as long as the parameters we provide in the fake message (U, price, weights, etc) are not too extreme.

Thus we get minted free tokens. I report this seperately, since I believe the fix proposed for the DoS issue would not suffice. The fix aims to protect users against getting fron-ran. It does not matter here, since the breaking point is the fact that no access control + signing of the message would allow us to easily verify it's signature and execute it, whatever it is. Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

I am not really sure what recommendation to give here, as the team does not with to apply access control for compatibility. Probably consider at which point of the call flow from sendAsset to submitMessage would do the message signing.

[PlamenTSV]

49 and #46 are the issues I mention, 49 being the invalid one and 46 being the valid one, which's fix would not work in this case.

[reednaa]

How are you circumventing verifySourceChainAddress(sourceIdentifier, fromApplication)? https://github.com/hats-finance/Catalyst-Exchange-0x3026c1ea29bf1280f99b41934b2cb65d053c9db4/blob/fba322fab023a9206183fb455e9f86facd550d8a/evm/src/CatalystChainInterface.sol#L478

https://github.com/hats-finance/Catalyst-Exchange-0x3026c1ea29bf1280f99b41934b2cb65d053c9db4/blob/fba322fab023a9206183fb455e9f86facd550d8a/evm/src/CatalystChainInterface.sol#L213-L216

From application is set here: https://github.com/catalystdao/GeneralisedIncentives/blob/2448d77e412216283ed75d8c3cbaa1270657f7b5/src/IncentivizedMessageEscrow.sol#L184

[PlamenTSV]

https://github.com/catalystdao/GeneralisedIncentives/blob/2448d77e412216283ed75d8c3cbaa1270657f7b5/src/IncentivizedMessageEscrow.sol#L184 Tracking back the calls, I see that the intended caller of submitMessage is the CatalystExchangeInterface. Does that mean that this is supposed to be the encoded msg.sender in the link above? Since the call is done through an interface, the msg.sender changes, so it should be the interface contract?

[reednaa]

I am not sure what the question is.

Anyone can all Generalised Incentives, it isn't meant for just Catalyst. Generalised Incentives doesn't do any security checks on message sender (msg.sender) and the destination application. (It does check that the calling incentive is valid though).

The sender of the message (msg.sender) will be passed to the application such that it can do the validation. That is what verifySourceChainAddress(sourceIdentifier, fromApplication) does.

[PlamenTSV]

Your argument is that in

        bytes memory messageWithContext = abi.encodePacked(
            bytes1(CTX_SOURCE_TO_DESTINATION),    // This is a sendPacket,
            messageIdentifier,              // A unique message identifier
            convertEVMTo65(msg.sender),     // Original sender
            destinationAddress,             // The address to deliver the (original) message to.
            incentive.maxGasDelivery,       // Send the gas limit to the other chain so we can enforce it
            message                         // The message to deliver to the destination.
        );

The encoded original sender will me the malicious user, thus https://github.com/hats-finance/Catalyst-Exchange-0x3026c1ea29bf1280f99b41934b2cb65d053c9db4/blob/fba322fab023a9206183fb455e9f86facd550d8a/evm/src/CatalystChainInterface.sol#L213-L216 would protect you against the attack. But if we another step before step 1 that being:

0. In CatalystExchangeInterface.sol the function sendCrossChainLiquidity lacks access control as well and does validation only for the passed incentive(routeDescription)'s max and min values and if the addresses are 65 bytes format. Thus if we begin the attack here, it will do GARP.submitMessage at the end, meaning that this time convertEVMTo65(msg.sender) would encode the interface contract address as the original sender and the checks in verifySourceChainAddress would be bypassed. This way we mimic the default behavior 1:1. Usually the contracts do Vault -> Interface -> IncentivesEscrow. We skip the Vault part (creation of token escrow and sending of source tokens) and begin at the Interface(CatalystExchangeInterface) since the access allows us so. Only thing the attacker needs are funds to cover for his gas/incentive values.

[reednaa]

How are you bypassing the security check onlyConnectedPool(channelId, fromVault):

https://github.com/hats-finance/Catalyst-Exchange-0x3026c1ea29bf1280f99b41934b2cb65d053c9db4/blob/fba322fab023a9206183fb455e9f86facd550d8a/evm/src/CatalystVaultVolatile.sol#L1031

Where the fromVault is set here: https://github.com/hats-finance/Catalyst-Exchange-0x3026c1ea29bf1280f99b41934b2cb65d053c9db4/blob/fba322fab023a9206183fb455e9f86facd550d8a/evm/src/CatalystChainInterface.sol#L362

[PlamenTSV]

Gg