Suricata

Reported by Communications Security Establishment (CSE):

## Summary

The `byte_math` keyword evaluator performs `val >>= rvalue;` on a `uint64_t` without checking that `rvalue < 64`, while the adjacent `LeftShift` case does guard this — the asymmetry indicates an oversight. Because `rvalue` can be sourced either from a rule literal (the Rust parser only bounds it to `0..=u32::MAX`) or, via `DETECT_BYTEMATH_FLAG_RVALUE_VAR`, directly from packet bytes captured by a preceding `byte_extract`, a remote peer can drive the shift amount to ≥ 64 and invoke C11 6.5.7p3 undefined behaviour on the per-packet detection hot path. On stock x86_64/AArch64 builds the hardware masks the shift count, yielding a *wrong* (non-zero) result that silently corrupts downstream `byte_test`/`isdataat` logic; under UBSan or trap-on-UB compiler modes it aborts the process. Exploitation requires a loaded rule that uses `byte_math … oper >>`.

## Affected Piece of Code

- **File:** `src/detect-bytemath.c`
- **Function / Location:** `DetectByteMathDoMatch()` ~L184-194, case `RightShift: val >>= rvalue;`
- **Subsystem:** detect-content-kw — content/pcre/byte_*/isdataat/replace keyword parsers and matchers

```c
/* /home/omuser/claude/suricata/src/detect-bytemath.c:180-194 */
            break;
        case Multiplication:
            val *= rvalue;
            break;
        case LeftShift:
            if (rvalue < 64) {
                val <<= rvalue;
            } else {
                val = 0;
            }
            break;
        case RightShift:
            val >>= rvalue;
            break;
    }
```

## The Bug

`DetectByteMathDoMatch()` (src/detect-bytemath.c:90) extracts up to 8 bytes from the packet payload into the local `uint64_t val` and then applies the rule-selected arithmetic operator using the `uint64_t rvalue` argument. The `switch (data->oper)` block at lines 165-194 already defends two operators against degenerate operands: `Division` checks `rvalue == 0` (lines 174-179) and `LeftShift` checks `rvalue < 64` (lines 184-189). The `RightShift` arm at lines 191-193, however, executes `val >>= rvalue;` unconditionally.

C11 §6.5.7p3 states: *"If the value of the right operand is negative or is greater than or equal to the width of the promoted left operand, the behavior is undefined."* Here the promoted left operand is `uint64_t` (width 64), so any `rvalue >= 64` is undefined behaviour — not merely an implementation-defined or saturating result.

`rvalue` is supplied by the caller `DetectEngineContentInspectionInternal()` (src/detect-engine-content-inspection.c) and originates from one of two places:

1. **Rule literal.** When the signature contains e.g. `rvalue 100`, the Rust keyword parser `SCByteMathParse` (rust/src/detect/byte_math.rs:210-240) parses the token as a `u32` and only enforces `0 <= rvalue <= u32::MAX`. The shift-specific validation at byte_math.rs:353-358 caps `nbytes` (≤ 4) for shift operators but never constrains `rvalue` itself. A rule author — or an attacker with rule-injection capability — can therefore load a signature whose literal right-shift amount is ≥ 64.

2. **Packet-derived variable.** When the rule uses `rvalue <name>` where `<name>` was previously declared by `byte_extract` or another `byte_math`, the parser sets `DETECT_BYTEMATH_FLAG_RVALUE_VAR` and `DetectByteMathSetup()` stores the referenced variable's `local_id` in `bmd->rvalue`. At match time, `DetectEngineContentInspectionInternal()` sees the flag and loads `rvalue = det_ctx->byte_values[bmd->rvalue]` (lines 612-617). That slot was populated moments earlier — for the same packet — by `DetectByteExtractDoMatch()` writing the raw extracted bytes into `det_ctx->byte_values[local_id]` (lines 590-593). A single payload byte of `0x40` therefore yields `rvalue == 64`, and `0xff` yields `rvalue == 255`, both fully attacker-controlled from the wire.

**Call chain (network packet → UB site):**

`FlowWorker()` [src/flow-worker.c:663] → `Detect()` [src/detect.c:2938] → `DetectFlow()` → `DetectRun()` [src/detect.c:106] → `DetectRulePacketRules()` [src/detect.c:747] → `DetectEnginePktInspectionRun()` [src/detect-engine.c:1910] → `DetectEngineInspectRulePayloadMatches()` [src/detect-engine.c:1865] → `DetectEngineInspectPacketPayload()` [src/detect-engine-payload.c:152] → `DetectEngineContentInspection()` [src/detect-engine-content-inspection.c:751] → `DetectEngineContentInspectionInternal()`.

Inside `DetectEngineContentInspectionInternal()`, while iterating the PMATCH `SigMatch` list for the signature, the engine first encounters the `DETECT_BYTE_EXTRACT` entry and calls `DetectByteExtractDoMatch()`, storing the packet-supplied value into `det_ctx->byte_values[local_id]` (lines 590-593). It then encounters the `DETECT_BYTEMATH` entry; because `DETECT_BYTEMATH_FLAG_RVALUE_VAR` is set it assigns `rvalue = det_ctx->byte_values[bmd->rvalue]` (lines 612-617) and calls `DetectByteMathDoMatch()` [src/detect-bytemath.c:90]. With `data->oper == RightShift`, control reaches line 192 and executes `val >>= rvalue;` on a `uint64_t` with no width check.

**Required state to trigger:**

- A loaded rule of the form `byte_extract:1,0,shift; byte_math:bytes 1, offset 0, oper >>, rvalue shift, result out, relative;` so that `DETECT_BYTEMATH_FLAG_RVALUE_VAR` is set and `bmd->oper == RightShift`; **and**
- An inbound packet whose first payload byte is ≥ `0x40` (decimal 64), so the extracted `shift` variable is ≥ 64.

The literal-rvalue path triggers identically with no variable: a rule containing `byte_math: … oper >>, rvalue 100, …` loads cleanly (Rust parser only bounds to `u32::MAX`), and every packet matching that rule executes `val >>= 100`.

**Practical effect.** On x86_64 and AArch64 the CPU masks the shift count to the low 6 bits, so `val >> 64` evaluates as `val >> 0` — i.e. `val` is returned unchanged instead of the mathematically correct `0`. This wrong value is then written to `det_ctx->byte_values[bmd->local_id]` and consumed by subsequent `byte_test`, `isdataat`, or `content` distance/within keywords in the same signature, flipping their match outcome. Under `-fsanitize=undefined`, or compilers/optimisation passes that exploit the UB, the operation may trap or be miscompiled.

**Vulnerability class:** network-reachable integer undefined-behaviour / detection-logic bypass.

## Reproduction Results

The trigger below is **analytically derived** from source review of `src/detect-bytemath.c`, `src/detect-engine-content-inspection.c`, and `rust/src/detect/byte_math.rs`. It was validated against the parser acceptance rules and the runtime call chain; it was **not** executed against a live UBSan build during this audit (no runtime environment available), so the UBSan output in step 5 is the expected, not observed, message.

1. Create a rules file `bm.rules` containing exactly one signature that uses a packet-derived variable as the right-shift amount:

   ```
   alert udp any any -> any 5555 (msg:"BM RSHIFT UB"; byte_extract:1,0,shift; byte_math:bytes 1, offset 0, oper >>, rvalue shift, result out, relative; byte_test:1,=,0,0,relative; sid:1000001; rev:1;)
   ```

   The Rust parser `SCByteMathParse` accepts this: `oper >>` → `ByteMathOperator::RightShift`; `rvalue shift` is a non-numeric string → `DETECT_BYTEMATH_FLAG_RVALUE_VAR` is set; `nbytes = 1 ≤ 4` passes the shift-operator `nbytes` check at byte_math.rs:353-358. `DetectByteMathSetup()` then resolves `shift` to the `byte_extract` `local_id`.

2. Start Suricata with this rule loaded, e.g.:

   ```
   suricata -c suricata.yaml -S bm.rules -i eth0
   ```

   (or `-r repro.pcap`). No other non-default configuration is required; the rule alone enables the code path.

3. Send a single UDP datagram to port 5555 whose payload is at least 2 bytes and whose first byte is `0x40` or larger. Concrete packet (Ethernet/IPv4/UDP, payload `40 00`):

   - Ethernet: `ff ff ff ff ff ff 00 11 22 33 44 55 08 00`
   - IPv4 (20 B, total len 30): `45 00 00 1e 00 01 00 00 40 11 00 00 0a 00 00 01 0a 00 00 02`
   - UDP (sport 1234, dport 5555, len 10): `04 d2 15 b3 00 0a 00 00`
   - Payload: `40 00`

   Or simply:

   ```
   printf '\x40\x00' | nc -u <sensor-ip> 5555
   ```

4. Execution path on receipt: `byte_extract` reads `payload[0] = 0x40` → `det_ctx->byte_values[0] = 64`. `byte_math` (relative, offset 0) reads `payload[1] = 0x00` into `val`, sets `rvalue = det_ctx->byte_values[0] = 64`, enters `case RightShift:` and executes `val >>= 64;` on a `uint64_t` — undefined behaviour per C11 §6.5.7p3.

5. Observation: build Suricata with `CFLAGS="-fsanitize=undefined"` and replay step 3; UBSan is expected to emit:

   ```
   detect-bytemath.c:192: runtime error: shift exponent 64 is too large for 64-bit type 'uint64_t'
   ```

   On a non-instrumented x86_64 build the CPU masks the count to `(64 & 63) = 0`, so `val` retains its original value instead of becoming `0`, producing an incorrect `byte_math` result that propagates to the following `byte_test` — a silent detection-logic error rather than a crash.

**Alternative trigger requiring no variable:** load

```
alert udp any any -> any 5555 (byte_math:bytes 1, offset 0, oper >>, rvalue 100, result out; sid:1000002;)
```

The literal `100` passes the parser (only bounded to `u32::MAX`) and every matching packet executes `val >>= 100`.

## Severity

**LOW** — Undefined behaviour (C11 §6.5.7p3) on a hot per-packet detection path.

- **UBSan / trap-on-UB builds:** remote denial-of-service (process abort) triggered by a single packet, conditional on a rule using `byte_math … oper >>` being loaded.
- **Stock GCC/Clang on x86_64/AArch64:** the hardware masks the shift count, so the practical effect is a wrong arithmetic result (e.g. `val >> 64` yields `val` instead of `0`). This can flip the outcome of subsequent `byte_test`/`isdataat`/`content` checks that consume the `byte_math` result variable — i.e. detection bypass or false positives for any signature built on that value.
- **No memory corruption** and **no information disclosure**.
- **Precondition:** the operator must have deployed (or an attacker with rule-write access must have injected) a rule using the `>>` operator with either a variable `rvalue` or a literal ≥ 64. No such rule ships in the default Suricata or Emerging Threats rulesets, which bounds real-world exposure.

## Suggested Fix

Mirror the `LeftShift` guard in the `RightShift` case so any shift amount ≥ 64 yields the mathematically correct result (`0`) instead of UB:

```c
--- a/src/detect-bytemath.c
+++ b/src/detect-bytemath.c
@@ -189,7 +189,11 @@ int DetectByteMathDoMatch(...)
             }
             break;
         case RightShift:
-            val >>= rvalue;
+            if (rvalue < 64) {
+                val >>= rvalue;
+            } else {
+                val = 0;
+            }
             break;
     }
```

Optionally, also tighten the Rust parser (`rust/src/detect/byte_math.rs`, around line 353) to reject a literal `rvalue >= 64` when `oper` is `LeftShift` or `RightShift`, so misconfigured rules fail at load time rather than silently saturating to `0`. The runtime guard above must remain regardless, because the variable-`rvalue` path is packet-controlled and cannot be validated at parse time.