Suricata

Reported by Communications Security Establishment (CSE):

## Summary

Suricata's general address parser (`DetectAddressParseString()`) accepts IPv6 dash-range notation such as `2001:db8::1-2001:db8::ff`, so a signature using that syntax loads successfully and may be classified as `SIG_TYPE_IPONLY`. The IP-only–specific parser (`IPOnlyCIDRItemParseSingle()`), however, has no handler for IPv6 dash ranges and rejects the token. The surrounding list parser (`IPOnlyCIDRListParse2()`) then takes its `error:` path but returns the partially-built `head` list instead of `NULL`, so the caller treats the parse as a success. The net effect is that an IP-only drop/alert rule whose address list contains an IPv6 dash range (preceded by at least one valid entry) is accepted with the IPv6 range silently stripped from the radix tree — all traffic from/to that range bypasses the rule, with no rule-load failure and no `--init-errors-fatal` abort.

## Affected Piece of Code

- **File:** `src/detect-engine-iponly.c`
- **Function / Location:** `IPOnlyCIDRItemParseSingle()` ~L339–L376 and `IPOnlyCIDRListParse2()` ~L690–L824
- **Subsystem:** detect-engine-addr — Detection address/port/iponly/proto handling

```c
src/detect-engine-iponly.c — IPOnlyCIDRItemParseSingle() IPv6 branch (no '-' handling):
   368          } else {
   369              r = inet_pton(AF_INET6, ip, &in6);
   370              if (r <= 0)
   371                  goto error;
   372  
   373              memcpy(dd->ip, &in6.s6_addr, sizeof(dd->ip));
   374              dd->netmask = 128;
   375          }
   376  

src/detect-engine-iponly.c — IPOnlyCIDRListParse2() error path returns partial list:
   808                  if (IPOnlyCIDRItemSetup(&subhead, address) < 0) {
   809                      IPOnlyCIDRListFree(subhead);
   810                      subhead = NULL;
   811                      goto error;
   812                  }
   813                  head = IPOnlyCIDRItemInsert(head, subhead);
   814              }
   815              n_set = 0;
   816          }
   817      }
   818  
   819      return head;
   820  
   821  error:
   822      SCLogError("Error parsing addresses");
   823      return head;
```

## The Bug

This is a **network-reachable logic bypass** rooted in a feature-parity gap between Suricata's two address parsers, compounded by an error-handling defect that converts a hard parse failure into a silent partial success.

### Root cause

`DetectAddressParseString()` in `src/detect-engine-address.c` (L515–532) explicitly supports the IPv6 dash-range form `aaaa::x-aaaa::y`: it splits on `'-'`, calls `inet_pton(AF_INET6, …)` on each half, and stores the resulting low/high pair. A signature whose source or destination address list contains `2001:db8::1-2001:db8::ff` therefore passes the primary address parse during `SigParse()` and is accepted as syntactically valid.

The IP-only fast-path parser `IPOnlyCIDRItemParseSingle()` in `src/detect-engine-iponly.c` does **not** mirror this. Its IPv6 branch (L339–376) handles only `addr/cidr` and bare-address forms; there is no `strchr(ip, '-')` case. When handed `2001:db8::1-2001:db8::ff`, control falls through to the bare-address `else` at L368, which calls `inet_pton(AF_INET6, "2001:db8::1-2001:db8::ff", &in6)`. `inet_pton` returns 0 for the malformed literal, the function jumps to `error:` and returns `-1`.

That `-1` propagates up through `IPOnlyCIDRItemSetup()` (L398) into `IPOnlyCIDRListParse2()` at L808–812, which frees the local `subhead` and jumps to `error:` at L821. Here is the second half of the bug: the `error:` label logs `"Error parsing addresses"` but then **returns `head`** — whatever portion of the list was successfully parsed before the failing token — instead of freeing it and returning `NULL`. Because the outer recursive caller (L695–699) and `IPOnlyCIDRListParse()` (L845–849) both treat any non-`NULL` return as success, the failure is swallowed as long as at least one earlier token in the same bracket scope parsed correctly.

### Call chain — phase A: rule load (operator-controlled config)

1. `SigLoadSignatures()` → `DetectEngineAppendSig()` (`detect-engine-loader.c:186` / `detect-parse.c:3595`)
2. → `SigInit()` (`detect-parse.c:3248`) → `SigInitHelper()` (`detect-parse.c:3016/3207`)
3. → `SigParse()` → `SigParseBasics()` → `SigParseAddress()` (`detect-parse.c:1112/1881`), which uses `DetectAddressParseString()` (`detect-engine-address.c:515–532`). This **accepts** the IPv6 dash-range `2001:db8::1-2001:db8::ff`, so the signature's address heads are populated and the rule is not rejected here.
4. `SigInitHelper()` → `SigValidateConsolidate()` (`detect-parse.c:3152`) → `SignatureSetType()` (`detect-parse.c:2978` / `detect-engine-build.c:1699`) → `SignatureIsIPOnly()` (`detect-engine-build.c:191`). With no payload/app-layer keywords present, the rule is assigned `s->type = SIG_TYPE_IPONLY`.
5. `SigValidateConsolidate()` (`detect-parse.c:3001–3008`) then invokes the IP-only parser: `IPOnlySigParseAddress()` (`detect-engine-iponly.c:869`) → `IPOnlyCIDRListParse()` (L838) → `IPOnlyCIDRListParse2()` (L660).
6. For input `[10.0.0.1,2001:db8::1-2001:db8::ff]`, the outer call strips the brackets and recurses on `10.0.0.1,2001:db8::1-2001:db8::ff`. The first token `10.0.0.1` parses successfully and is inserted into `head`. The second token reaches `IPOnlyCIDRItemSetup()` (L398) → `IPOnlyCIDRItemParseSingle()` (L219), enters the IPv6 branch (L339–376) with no `'-'` handling, fails `inet_pton` at L369, and returns `-1`.
7. Back in `IPOnlyCIDRListParse2()` L808–812: `subhead` is freed, `goto error` → L821–823 returns the partial `head` (containing only `10.0.0.1`). The outer recursion at L695–699 sees non-`NULL` and treats it as success; `IPOnlyCIDRListParse()` (L845–849) sees non-`NULL` and returns `0`; `IPOnlySigParseAddress()` returns `0`; `SigValidateConsolidate()` returns `1`. **The rule is accepted.**
8. Later, `SigAddressPrepareStage1()` → `IPOnlyAddSignature()` (`detect-engine-build.c:1972`) and `IPOnlyPrepare()` (`detect-engine-build.c:1992` / `detect-engine-iponly.c:1142`) build the IP-only radix trees from `s->init_data->cidr_src` / `cidr_dst`. Those lists contain **only** `10.0.0.1`; the IPv6 range `2001:db8::1-2001:db8::ff` is never inserted into `io_ctx->tree_ipv6src` / `tree_ipv6dst`.

### Call chain — phase B: packet path (attacker-controlled)

1. `FlowWorker` → `Detect` / `DetectFlow` / `DetectNoFlow` (`detect.c:2846/2860`) → `DetectRun()` (`detect.c:106`)
2. → `DetectRunInspectIPOnly()` (`detect.c:124/501`) → `IPOnlyMatchPacket()` (`detect-engine-iponly.c:1000`).
3. For an IPv6 packet with `src = 2001:db8::10`, `SCRadix6TreeFindBestMatch()` at L1013 is queried against `io_ctx->tree_ipv6src`. Because the range was never inserted, the lookup returns `NULL`.
4. At L1028, `if (src == NULL || dst == NULL) SCReturn;` exits with no match. The IP-only drop/alert rule **never fires** for any address inside the dropped IPv6 range.

### Required field values / preconditions

- The signature must be IP-only (no `content`, app-layer, or other keywords that would remove `SIG_TYPE_IPONLY` classification).
- The signature's source or destination address list must contain **at least one successfully-parsed entry before** an IPv6 dash-range entry at the same bracket nesting level, e.g. `[10.0.0.1,2001:db8::1-2001:db8::ff]`. Order matters: `[2001:db8::1-2001:db8::ff,10.0.0.1]` fails while `head` is still `NULL`, so the inner `IPOnlyCIDRListParse2()` returns `NULL` and the rule is rejected outright — the bypass does not occur in that ordering.
- The attacker then sends any IPv6 packet whose source (or destination, depending on which side the list is on) lies inside the dropped range.

The rule content is operator-controlled, not attacker-controlled; the attacker-controlled input is the IPv6 packet that exploits the resulting gap in coverage.

## Reproduction Results

The trigger below is **analytically derived** from source review of the call chain above. It has not been executed against a live build in this audit environment, but every step maps to a concrete, line-numbered code path with no intervening guard that would block it; confidence in reproducibility is high.

1. **Create the rules file** `/tmp/iponly.rules` containing exactly:

   ```
   drop ip [10.0.0.1,2001:db8::1-2001:db8::ff] any -> any any (msg:"block listed sources"; sid:1000001; rev:1;)
   ```

2. **Start Suricata** with this rule in any capture mode, e.g.:

   ```
   suricata -c suricata.yaml -S /tmp/iponly.rules -r bypass.pcap
   ```

   (or `-i eth0` in IPS mode). At startup, observe two `SCLogError` lines:

   ```
   address parsing error "2001:db8::1-2001:db8::ff"   (detect-engine-iponly.c:404)
   Error parsing addresses                            (detect-engine-iponly.c:822)
   ```

   …but the rule is **not** rejected — the loader reports `1 rule successfully loaded`. Even with `--init-errors-fatal`, the engine does not abort, because `IPOnlySigParseAddress()` returned `0`.

3. **Send a packet from inside the IPv6 range.** Craft an IPv6/ICMPv6 echo-request with `src=2001:db8::10`, `dst=2001:db8::ffff` (any destination). Raw L3 bytes (48 bytes) for a pcap with linktype `RAW`:

   ```
   60 00 00 00  00 08 3a 40
   20 01 0d b8  00 00 00 00  00 00 00 00  00 00 00 10   (src 2001:db8::10)
   20 01 0d b8  00 00 00 00  00 00 00 00  00 00 ff ff   (dst 2001:db8::ffff)
   80 00 2d 39  00 00 00 00                             (ICMPv6 type=128 code=0 cksum=0x2d39 id=0 seq=0)
   ```

   Or with scapy:

   ```python
   wrpcap('bypass.pcap', [IPv6(src='2001:db8::10', dst='2001:db8::ffff')/ICMPv6EchoRequest()])
   ```

4. **Expected vs. actual.**
   - *Expected (per the operator-written rule):* the packet matches `sid:1000001` → drop/alert.
   - *Actual:* `IPOnlyMatchPacket()` looks up `2001:db8::10` in `tree_ipv6src`, finds nothing (the range was discarded), and returns at L1028 with no match. No alert appears in `eve.json` / `fast.log`; in IPS mode the packet is forwarded.
   - *Control:* a packet with IPv4 `src 10.0.0.1` **does** alert/drop, proving the rule is loaded and active for the surviving list entry only.

**Ordering caveat for reproduction:** the IPv6 dash-range must **not** be the first element of its bracket list. `[2001:db8::1-2001:db8::ff,10.0.0.1]` causes the inner `IPOnlyCIDRListParse2()` to hit `error:` while `head` is still `NULL` → returns `NULL` → the rule is rejected, so the bypass does not occur in that ordering.

## Severity

**MEDIUM** — Detection bypass. An IP-only `drop`/`alert`/`reject` rule that the operator believes covers an IPv6 address range (written in `aaaa::1-aaaa::n` form alongside other addresses) is loaded with that range silently stripped from the IP-only radix tree. All traffic from or to addresses inside the IPv6 range evades the rule entirely — no alert, and no drop in IPS mode. Two `SCLogError` lines are printed at startup, but the rule is still accepted (the return value is success), so `--init-errors-fatal` does not catch it and automated or long-running deployments are unlikely to notice. There is no crash and no memory corruption; this is a pure policy/detection bypass conditioned on a specific — but documented-as-valid — operator rule syntax. The requirement for a particular operator-written rule shape (mixed list with the IPv6 range not first) keeps this below HIGH.

## Suggested Fix

Two complementary changes in `/home/omuser/claude/suricata/src/detect-engine-iponly.c`:

**(1) Make the `error:` path of `IPOnlyCIDRListParse2()` actually fail** so callers reject the signature. This also hardens against any other future per-token parse failure, and the analogous path at L751–755:

```c
@@ -821,3 +821,5 @@ static IPOnlyCIDRItem *IPOnlyCIDRListParse2(
 error:
     SCLogError("Error parsing addresses");
-    return head;
+    if (head != NULL)
+        IPOnlyCIDRListFree(head);
+    return NULL;
```

**(2) Add the missing IPv6 dash-range branch to `IPOnlyCIDRItemParseSingle()`** so the IP-only parser matches `DetectAddressParseString()` (`detect-engine-address.c:515–532`). Insert before the final `else` at L368:

```c
        } else if ((ip2 = strchr(ip, '-')) != NULL) {
            /* 2001::1-2001::4 range format */
            *ip2++ = '\0';
            struct in6_addr a, b;
            if (inet_pton(AF_INET6, ip,  &a) <= 0) goto error;
            if (inet_pton(AF_INET6, ip2, &b) <= 0) goto error;
            if (AddressIPv6Gt((Address *)&a, (Address *)&b)) goto error;
            /* expand [a,b] into a minimal set of CIDR blocks and link them
             * via dd / dd->next, mirroring the IPv4 InsertRange() logic. */
            return IPOnlyInsertIPv6Range(pdd, dd, &a, &b);
        } else {
```

If implementing the full IPv6 CIDR-cover algorithm is out of scope for a hotfix, **fix (1) alone is sufficient to close the bypass**: it turns the silent partial-success into a hard rule-load failure, which `--init-errors-fatal` and normal rule-error accounting will then surface to the operator.