c4-bot-4 commented 4 months ago

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph/blob/032396f701be935b04a7e5cf3cb40a0136259dbc/contracts/tapiocaOmnichainEngine/TapiocaOmnichainReceiver.sol#L286-L292

Vulnerability details

Impact

Anyone not in control of their address on all chains TapToken is deployed on can have all their TAP stolen.

Proof of Concept

TapToken has a feature where you can trigger a transfer cross chain using the LZCompose functionality. If you send a cross chain message with the type, MSG_REMOTE_TRANSFER that will transfer tokens from the destination chain to another chain. I.e. "pull" tokens from the destination chain.

The issue is that there is an assumption made in the TapiocaOmnichainReceiver::_internalTransferWithAllowance:

File: tapioca-periph/contracts/tapiocaOmnichainEngine/TapiocaOmnichainReceiver.sol

286:    function _internalTransferWithAllowance(address _owner, address srcChainSender, uint256 _amount) internal {
287:        if (_owner != srcChainSender) {
288:            _spendAllowance(_owner, srcChainSender, _amount);
289:        }
290:
291:        _transfer(_owner, address(this), _amount);
292:    }

Here, if the srcChainSender, msg.sender on the source chain, the one initiating the transfer is the same address the transfer will not check any allowance.

This makes an assumption about the holders of TapTokens. Since they must control their address on all chain TapToken is deployed on. As shown in the wintermute hack this is not always the case. Some older SAFE wallets can have their address taken on other chains.

Any user having TAP needs to be aware of this and know the risks with holding tap in different smart wallets.

Tools Used

Manual audit

Recommended Mitigation Steps

Consider actually requiring an allowance for users to do this, even if the address is the same. Or simply don't allow cross chain "pull" of tokens.

Assessed type

Other

c4-sponsor commented 4 months ago

cryptotechmaker marked the issue as disagree with severity

cryptotechmaker commented 4 months ago

Low/Informational; we are aware that contracts might not have the same address on different chains and we'll have that warning when interacting with the protocol