hats-finance / illuminex-0x0bb4aa1f58719707405c231fcdf0b405714799cf

0 stars 1 forks source link

VaultBitcoinWallet : `startRefuelTxSerializing` & `startOutgoingTxSerializing` could get blocked due to `toggleRelayer` calling #86

Open hats-bug-reporter[bot] opened 2 months ago

hats-bug-reporter[bot] commented 2 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x6519fb156e20928e93ebfcd612ebb3c5cb23de77a5ce5c07d7734070f1f8f34b Severity: high

Description: Description

Relayer is responsible for most of the critical functionlities such as startRefuelTxSerializing, startOutgoingTxSerializing, finaliseOutgoingTxSerializing and the operations related to serializing and processing.

The relayer can be set by the owner which can be seen in the abstract contract AllowedRelayers.

AllowedRelayers.sol#L18-L33

    constructor() {
        relayersWhitelistEnabled = true;
        _toggleRelayer(msg.sender);
    }

    function _toggleRelayer(address _relayer) internal {
        relayers[_relayer] = !relayers[_relayer];
    }

    function toggleRelayersWhitelistEnabled() public onlyOwner {
        relayersWhitelistEnabled = !relayersWhitelistEnabled;
    }

    function toggleRelayer(address _relayer) public onlyOwner {
        _toggleRelayer(_relayer);
    }

Please note that the toggleRelayer function is restricted to be called ONLY by the owner.

if we see the functions, startRefuelTxSerializing and startOutgoingTxSerializing, they are called by the relayer. At the end of the fucntion, toggleRelayer is called. Note, here the msg.sender is relayer address.

As shown in the codes, the toggleRelayer has onlyOwner restriction. so they call will revert from above functions.

Impact

startRefuelTxSerializing and startOutgoingTxSerializing functionlaites would be blocked and nothing can be processed further.

Revised Code File (Optional)

toggleRelayer function calling in both startRefuelTxSerializing and startOutgoingTxSerializing can be removed. From our investigation we could not see any advantage for having it.

aktech297 commented 2 months ago

@party-for-illuminati why this is invalid ?

party-for-illuminati commented 2 months ago

@party-for-illuminati why this is invalid ?

It won't revert, please test it

aktech297 commented 2 months ago

@party-for-illuminati why this is invalid ?

It won't revert, please test it

ya. it is not reverting. the ownership is transferred before calling.