Suricata

Summary¶

IP-only rules with a dotted netmask match 0.0.0.0 instead of the network in the rule.

IPOnlyCIDRItemParseSingle() in src/detect-engine-iponly.c throws away the network address of any IPv4 address written with a dotted netmask. For example, it stores 203.0.113.0/255.255.255.0 as 0.0.0.0/24. Suricata uses this function for IP-only rules, which are rules that only match on addresses, ports and protocol. So a rule like drop ip 203.0.113.0/255.255.255.0 any -> $HOME_NET any (sid:1;) never matches traffic from 203.0.113.0/24, and matches traffic from 0.0.0.0/24 instead. An attacker in the network named in the rule can send traffic through the sensor without being dropped or alerted on. Suricata loads the rule without any warning. The same happens when an address variable like HOME_NET in suricata.yaml uses a dotted netmask: every IP-only rule that uses that variable then matches the wrong network.

Root cause¶

https://github.com/OISF/suricata/blob/bc48b4f44c7d3bdd06bf670405a2f016bb1157a1/src/detect-engine-iponly.c#L270-L304

uint32_t netmask = 0;
...
if ((strchr (mask, '.')) == NULL) {
    /* 1.2.3.4/24 format */
    ...
    dd->netmask = cidr;
    netmask = CIDRGet(cidr);
} else {
    /* 1.2.3.4/255.255.255.0 format */
    r = inet_pton(AF_INET, mask, &in);
    if (r <= 0)
        goto error;
    int cidr = CIDRFromMask(in.s_addr);
    if (cidr < 0)
        goto error;
    dd->netmask = (uint8_t)cidr;
}
r = inet_pton(AF_INET, ip, &in);
if (r <= 0)
    goto error;
dd->ip[0] = in.s_addr & netmask;

The code keeps two values for the mask. dd->netmask is the prefix length, for example 24. The local netmask is the 32-bit mask used on line 304 to clear the host bits of the address. netmask starts at 0 on line 270. The CIDR branch sets both values (lines 285-286). The dotted branch only sets dd->netmask (line 297) and never sets netmask, so it stays 0. Line 304 then ANDs the address with 0, and dd->ip[0] is always 0.0.0.0. The prefix length is still right, so 203.0.113.0/255.255.255.0 becomes 0.0.0.0/24.

The bug was introduced in Suricata 6.0.5, when the masking on line 304 was added (commit 51d4e0dced, "detect/iponly: fix netmask handling"). It is present on main, main-8.0.x and main-7.0.x.

Rules that are not IP-only parse the same string with DetectAddressParseString(), which sets netmask = in.s_addr correctly. So 203.0.113.0/255.255.255.0 works in a rule with content, but not in an IP-only rule.

Suggested fix¶

Set netmask in the dotted branch, the same way DetectAddressParseString() does:

    dd->netmask = (uint8_t)cidr;
    netmask = in.s_addr;
}

It would also help to add a unit test with a non-zero dotted mask, such as 1.2.3.4/255.255.255.0 expecting 1.2.3.0/24. The existing test IPOnlyTestBug5168v1 uses 1.2.3.64/0.0.0.0, where 0.0.0.0 is the correct answer, so it does not catch this bug.

Reproducer¶

The attached test.rules, input.pcap and test.yaml make up a suricata-verify test (iponly-dotted-netmask-01):

• test.rules: the IP-only rule drop ip 203.0.113.0/255.255.255.0 any -> any any.
• input.pcap: two UDP packets to 192.168.1.10, one from 203.0.113.5 and one from 0.0.0.5.
• test.yaml: runs Suricata in simulated IPS mode and checks that the packet from 203.0.113.5 is blocked and that the packet from 0.0.0.5 is not.

On affected code both checks fail:

===> iponly-dotted-netmask-01: Sub test #1: FAIL : expected 1 matches; got 0 for filter {'count': 1, 'match': {'event_type': 'alert', 'alert.signature_id': 1, 'alert.action': 'blocked', 'src_ip': '203.0.113.5'}}
===> iponly-dotted-netmask-01: Sub test #2: FAIL : expected 0 matches; got 1 for filter {'count': 0, 'match': {'event_type': 'alert', 'src_ip': '0.0.0.5'}}

min-version: 7 in test.yaml is a placeholder and should be set to the first release that has the fix.

Credit¶

Please credit Anthropic and Ada Logics (Adam Korczynski).