sherlock-audit / 2024-02-tapioca-judging

3 stars 2 forks source link

bin2chen - Multiple lzCompose messages did not verify the legality of _srcChainSender #109

Closed sherlock-admin4 closed 4 months ago

sherlock-admin4 commented 4 months ago

bin2chen

high

Multiple lzCompose messages did not verify the legality of _srcChainSender

Summary

In the current protocol, several modules only require the _toeComposeMsg to execute. However, there is no validation of the legality of _srcChainSender. As a result, anyone can construct a _toeComposeMsg to execute arbitrary lzCompose information.

Vulnerability Detail

Let's take TOFTMarketReceiverModule.marketRemoveCollateralReceiver as an example. This module's parameter is only _data = _toeComposeMsg.

function marketRemoveCollateralReceiver(bytes memory _data) public payable {
    /// @dev decode received message
    MarketRemoveCollateralMsg memory msg_ = TOFTMsgCodec.decodeMarketRemoveCollateralMsg(_data);
}

Without passing _srcChainSender and validating its legitimacy, anyone can front-run construct a compose msg for execution.

If permit information for v, r, and s is required for composing, it can be obtained from the mempool.

Impact

The lack of a _srcChainSender security check allows anyone to modify the compose msg for execution.

Code Snippet

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/tOFT/modules/TOFTMarketReceiverModule.sol#L161

Tool used

Manual Review

Recommendation

It is recommended that all modules check _srcChainSender. For instance, consider adding something similar to _internalTransferWithAllowance(msg_.receiver, srcChainSender, msg_.amount);. This recommendation applies to modules such as:

Duplicate of #14

0xRektora commented 4 months ago

This is a valid issue that is different from #67 should be deduped

cryptotechmaker commented 3 months ago

CC @nevillehuang

nevillehuang commented 3 months ago

@cryptotechmaker I believe this is a duplicate of #111 and family, all points to unvalidated srcChainSender

sherlock-admin4 commented 3 months ago

The protocol team fixed this issue in PR/commit https://github.com/Tapioca-DAO/TapiocaZ/pull/189; https://github.com/Tapioca-DAO/Tapioca-bar/pull/377.

0xRektora commented 3 months ago

This also fixes #140