Attacker can drain all tap token from victim's wallet using function `BaseTapiocaOmnichainEngine::transferFrom()`

[c4-bot-9]

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph/blob/032396f701be935b04a7e5cf3cb40a0136259dbc/contracts/tapiocaOmnichainEngine/BaseTapiocaOmnichainEngine.sol#L60-L74

Vulnerability details

Description

The function BaseTapiocaOmnichainEngine::transferFrom() is a modified version of the well-known function ERC20::transferFrom() with slight alterations to accommodate the pearlmit contract.

function transferFrom(address from, address to, uint256 value) public virtual override returns (bool) {
    address spender = _msgSender();
    // If the allowance on this contract is not met, attempt a transferFrom via Pearlmit.
    if (allowance(from, spender) < value) {
        // _transfer(from, to, value);
        bool isErr = pearlmit.transferFromERC20(from, to, address(this), value);
        if (isErr) revert BaseTapiocaOmnichainEngine_NotValid();
    } else {
        // If the allowance on this contract is met, carry out a normal transferFrom.
        _spendAllowance(from, spender, value);
        _transfer(from, to, value);
    }

    return true;
}

In the scenario where the allowance of the from address with respect to the msg.sender is less than the transfer amount value, the function invokes pearlmit.transferFromERC20(from, to, address(this), value) to execute the token transfer. However, a vulnerability arises as there is no check for permission between the from address and the msg.sender.

The attacker can arbitrarily set the from address to one that has:

• Approved the contract BaseTapiocaOmnichainEngine in the pearlmit contract, and
• BaseTapiocaOmnichainEngine.allowance(from, pearlmit) > 0.

Additionally, the attacker can set the to address to their own address. By doing so, all tokens from the from address will be stolen by the attacker.

Impact

Attacker can steal Tap tokens from other users

Tools Used

Manual review

Recommended Mitigation Steps

Consider removing the logic with the pearlmit contract in the function transferFrom()

Assessed type

Access Control

[c4-sponsor]

cryptotechmaker marked the issue as disagree with severity

[cryptotechmaker]

Medium; the attack is possible only if the victim gives approval to the attacker

[c4-judge]

dmvt marked the issue as duplicate of #172

[c4-judge]

dmvt changed the severity to 2 (Med Risk)

[c4-judge]

dmvt marked the issue as satisfactory