hats-finance / illuminex-0x0bb4aa1f58719707405c231fcdf0b405714799cf

0 stars 1 forks source link

The onlyRelayer modifier does not check correctly #32

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

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

Github username: -- Twitter username: -- Submission hash (on-chain): 0x89233d417492fd61fba4a61c8d49d18d35f6cb508159a56d4e433d1475f33025 Severity: medium

Description: Description\ An attacker can bypass the onlyRelayer modifier with an address that is not a relayer because the modifier doesn't revert if relayersWhitelistEnabled is false and if !relayers[msg.sender]. It's only revert if relayersWhitelistEnabled is true and !relayers[msg.sender].

Attack Scenario\

  1. The owner calls toggleRelayersWhitelistEnabled and set relayersWhitelistEnabled to false.
  2. The attacker can bypass the onlyRelayer modifier with an address that is not a relayer. He can therefore call functions that use the modifier.
  3. The attacker calls ackTransaction, ackAnchorBlock, startRefuelTxSerializing, finaliseRefuelTxSerializing, startOutgoingTxSerializing, finaliseOutgoingTxSerializing, copyParentInputs, enrichOutgoingTransaction functions.

Attachments

  1. Proof of Concept (PoC) File

    modifier onlyRelayer() {
    if (relayersWhitelistEnabled && !relayers[msg.sender]) {
        revert("NRL");
    }
    _;
    }
  2. Revised Code File (Optional)

You must make these changes on the modifier:

    modifier onlyRelayer() {
    if (!relayersWhitelistEnabled || !relayers[msg.sender]) {
        revert("NRL");
    }
    _;
    }
rotcivegaf commented 3 months ago

I think this is intentional, if relayersWhitelistEnabled: false the "white list it's no enable"