Unchecked `_claimTwpTapRewardsReceiver` `_srcReceiver` allows Permit Front-runner to steal rewards to self

[hats-bug-reporter[bot]]

Github username: @GalloDaSballo Twitter username: GalloDaSballlo Submission hash (on-chain): 0xcf8d7e8ab9044a856ce5f4bab70fca08cee234c2aa129fcc1daf5991b3d649e9 Severity: high

Description:

Impact

_claimTwpTapRewardsReceiver only checks that the approval was given, but doesn't check who's performing the current call

    function _claimTwpTapRewardsReceiver(bytes memory _data) internal virtual twTapExists {
        ClaimTwTapRewardsMsg memory claimTwTapRewardsMsg_ = TapTokenCodec.decodeClaimTwTapRewardsMsg(_data);
        // Claim rewards, make sure to have approved this contract on TwTap. /// @audit approval could be front-run
        uint256[] memory claimedAmount_ = twTap.claimRewards(claimTwTapRewardsMsg_.tokenId); 
        address owner = twTap.ownerOf(claimTwTapRewardsMsg_.tokenId);
        // Clear the allowance, claimRewards only does an allowance check.
        pearlmit.clearAllowance(owner, 721, address(twTap), claimTwTapRewardsMsg_.tokenId);

POC

This, in combination with TapiocaOmnichainSender.sendPacket being a open function, which allows passing arbitrary data via _composeMsg means that a malicious actor could:

• Detect the xChain Rewards claim Tx, with permit
• Broadcast the permit themselves, making twTap.claimRewards work
• Write a ClaimTwTapRewardsMsg which sets the claimTwTapRewardsMsg_.sendParam[sendParamIndex].sendParam.to to themselves
• Steal all rewards

Note

I believe that in general the compose payload from lzSend needs to be validated, or alternatively all receiver functions like _claimTwpTapRewardsReceiver need to validate who the original message sender was in relation to the operation they are performing

On Severity

Per Hats classification, theft of user funds or yield is High Severity

Mitigation

This type of "injected" attack could be best solved by recursively sanitizing the composeMessage payloads to ensure that:

• All payloads belong to a recognize format and struct
• Data within the payloads is safe

Alternatively, the receiving functions should be limited to owner or approved callers, which is consistent with how checks are done in other functions, although I believe is a lot more difficult to test and verify E2E

[0xRektora]

Front-run attacks are conveniently suppressed within Tapioca contract. All xChain calls are meant to meant executed only for the intended user. I'd invalidate this except if there's a PoC demonstrating the contrary.

We’re checking for an approved executor. If it's done on the same chain, it's either the user's address or the user's desired address (Magnetar), if it's done cross chain, it's still using the desired address See https://github.com/Tapioca-DAO/tapioca-periph/blob/dev/contracts/tapiocaOmnichainEngine/extension/TapiocaOmnichainExtExec.sol#L107 and https://github.com/Tapioca-DAO/tapioca-periph/blob/dev/contracts/tapiocaOmnichainEngine/TapiocaOmnichainReceiver.sol#L294) Which for reference, would be a Tapioca contract that also verifies the identity of the src chain sender. The hashedData comes in handy for xChain operations, where we want to ensure that the caller at dst chain is the src chain sender, as shown on the previous link on TapiocaOmnichainExtExec::pearlmitApproval().

Because we’re expecting the hashedData to be the srcChainSender, as signed by the user on the src chain, and also overwriting the batch owner to be the LZ srcChainSender (redundant with hashedData but we kept it) , if the signature doesn’t match the call will be reverted