Missing Event Emission in `transferAllowance` Function When Using External Router

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-06-thorchain/blob/e3fd3c75ff994dce50d6eb66eb290d467bd494f5/chain/ethereum/contracts/THORChain_Router.sol#L165

Vulnerability details

Impact

The transferAllowance function does not emit an event when router != address(this). This omission leads to a lack of transparency and tracking for allowance deductions and transfers made from the vault in this function call. The absence of an event contradicts also the code comment that states events are emitted for all outgoing transfers, as the network will not be alerted to these critical transactions.

Proof of Concept

The relevant part of the code is as follows:

function transferAllowance(
    address router,
    address newVault,
    address asset,
    uint amount,
    string memory memo
) external nonReentrant {
    if (router == address(this)) {
        _adjustAllowances(newVault, asset, amount);
        emit TransferAllowance(msg.sender, newVault, asset, amount, memo);
    } else {
        _routerDeposit(router, newVault, asset, amount, memo);
    }
}

When router != address(this), the function calls _routerDeposit but does not emit any event to account for the allowance deducted and transfers made. Though understandable, an event was emitted after iROUTER(_router).depositWithExpiry(_vault, _asset, _amount, _memo, type(uint).max) function call; However, no event was emitted to account for the allowance deducted and also transfers made from the vault.

Tools Used

Manual

Recommended Mitigation Steps

Emit event to account for the allowance deducted and also transfers made from the vault. or emit the TransferAllowance event regardless of whether the router is the current contract or an external

Assessed type

ERC20