Project

General

Profile

Actions

Security #8837

open
SB GL

ssh: miscalculation due to oversized KEXINIT causes parser desync

Security #8837: ssh: miscalculation due to oversized KEXINIT causes parser desync

Added by Shivani Bhardwaj 29 days ago. Updated about 2 hours ago.

Status:
Assigned
Priority:
Normal
Target version:
Affected Versions:
Label:
CVE:
Git IDs:
Severity:
LOW
Disclosure Date:
GHSA:

Description

Reported by Communications Security Establishment (CSE):

## Summary

When the Rust SSH application-layer parser encounters an incomplete `SSH_MSG_KEXINIT` record whose declared body length meets or exceeds `SSH_MAX_REASSEMBLED_RECORD_LEN` (65535 bytes) while hassh fingerprinting is enabled, it overwrites the correctly-computed `record_left` skip counter with a value that is too large by exactly the number of body bytes already present in the current input slice. Because the function then returns `AppLayerResult::ok()` — telling the app-layer framework those body bytes were fully consumed — the next invocation over-skips into the following record's header, permanently desynchronising the SSH record framer for that flow direction. The result is that hassh is never computed, `ssh.hassh*` rule keywords cannot match, and the parser typically errors out and stops inspecting the direction. This is a network-reachable detection bypass with no memory-safety impact.

## Affected Piece of Code

- **File:** `rust/src/ssh/ssh.rs`
- **Function / Location:** `SSHState::parse_record()` ~L276–300
- **Subsystem:** rust-ike-ssh — IKE and SSH parsers

```rust
rust/src/ssh/ssh.rs:
   276                              let remlen = rem.len() as u32;
   277                              hdr.record_left = head.pkt_len - 2 - remlen;
   278                              //header with rem as incomplete data
   279                              match head.msg_code {
   280                                  parser::MessageCode::NewKeys => {
   281                                      hdr.flags = SSHConnectionState::SshStateFinished;
   282                                  }
   283                                  parser::MessageCode::Kexinit if hassh_is_enabled() => {
   284                                      // check if buffer is bigger than maximum reassembled packet size
   285                                      hdr.record_left = head.pkt_len - 2;
   286                                      if hdr.record_left < SSH_MAX_REASSEMBLED_RECORD_LEN as u32 {
   287                                          // saving type of incomplete kex message
   288                                          hdr.record_left_msg = parser::MessageCode::Kexinit;
   289                                          return AppLayerResult::incomplete(
   290                                              (il - rem.len()) as u32,
   291                                              head.pkt_len - 2,
   292                                          );
   293                                      } else {
   294                                          SCLogDebug!("SSH buffer is bigger than maximum reassembled packet size");
   295                                          self.set_event(SSHEvent::LongKexRecord);
   296                                      }
   297                                  }
   298                                  _ => {}
   299                              }
   300                              return AppLayerResult::ok();
```

## The Bug

### Call chain

A TCP packet for an SSH flow is passed up through `StreamTcpReassembleAppLayer` → `AppLayerParserParse()` (`src/app-layer-parser.c`) → the registered `parse_ts` / `parse_tc` callback `ssh_parse_request()` / `ssh_parse_response()` (`rust/src/ssh/ssh.rs:455-483`, registered in `SCRegisterSshParser()` at L549-550) → `SSHState::parse_record()` (`ssh.rs:135-326`).

Inside `parse_record()`, once the banner has been consumed, `parser::ssh_parse_record()` (`parser.rs:133`) is called on the remaining input. For a record whose body is not yet fully present it returns `Err(nom::Err::Incomplete(_))`. The code then falls back to `parser::ssh_parse_record_header()` (`parser.rs:118`), which succeeds because only the 6-byte header (4-byte `pkt_len`, 1-byte `padding_len`, 1-byte `msg_code`) is required; it yields `head` plus `rem`, the slice of body bytes that *are* already in the current input.

### Faulty state update

At L276-277 the parser computes the correct number of body bytes that still need to arrive in future stream slices:

```
hdr.record_left = head.pkt_len - 2 - remlen
```

This is the value the skip block at L146-172 will subtract from the *next* input slice. It is correct because the current slice already contains `remlen` body bytes that — if the function returns `AppLayerResult::ok()` — the app-layer framework will treat as consumed.

The `match` arm at L283 then fires when `head.msg_code == MessageCode::Kexinit` (byte `0x14`) **and** `hassh_is_enabled()` is true. `hassh_is_enabled()` returns true when `app-layer.protocols.ssh.hassh: yes` is set in `suricata.yaml` (`src/app-layer-ssh.c:91-106`) or, with hassh on `auto`/unset, when any loaded rule uses an `ssh.hassh*` keyword (`rust/src/ssh/detect.rs:182/204/226/248` call `SCSshEnableHassh()`).

Inside that arm, L285 **unconditionally overwrites** `hdr.record_left` with `head.pkt_len - 2` — the full body length, with no subtraction of `remlen`.

For records with `pkt_len - 2 < SSH_MAX_REASSEMBLED_RECORD_LEN (65535)` this overwrite is harmless: the branch at L286-292 returns `AppLayerResult::incomplete((il - rem.len()) as u32, head.pkt_len - 2)`, which tells the framework that only the bytes up to (but not including) `rem` were consumed. The framework will re-deliver the `remlen` body bytes on the next call, so a `record_left` of `head.pkt_len - 2` is consistent.

For an oversized KEXINIT — `head.pkt_len - 2 >= 65535`, i.e. `pkt_len >= 65537` — the `else` branch at L293-296 is taken instead. It only logs and sets `SSHEvent::LongKexRecord`, then falls through to `return AppLayerResult::ok()` at L300. `AppLayerResult::ok()` tells the framework that the **entire** current input slice, including the `remlen` body bytes, has been consumed and should not be re-delivered. But `hdr.record_left` was left at `head.pkt_len - 2`, which is exactly `remlen` bytes larger than the number of body bytes that will actually still arrive.

### Consequence

On the next call to `parse_record()` for this direction, the skip block at L146-172 uses the inflated `hdr.record_left` to slice off the front of the new input. It therefore discards `remlen` bytes too many, eating into the 6-byte header of the record that follows the oversized KEXINIT. From that point the SSH record framer for this direction is permanently desynchronised: subsequent `pkt_len` fields are read from the middle of record bodies, KEXINIT/NEWKEYS are never recognised, hassh for this direction is never computed, and `ssh.hassh` / `ssh.hassh.string` (or `.server` variants) rule keywords never match. In practice the mis-framed data quickly fails `ssh_parse_record_header()`'s validity checks, the parser sets `SSHEvent::InvalidRecord`, returns `AppLayerResult::err()`, and the app-layer framework stops invoking the SSH parser for that direction of the flow.

Vulnerability class: **network-reachable logic-bypass**.

## Reproduction Results

**Prerequisite config** — enable hassh by **either**:

  (a) `suricata.yaml`: `app-layer.protocols.ssh.hassh: yes`, **or**
  (b) load any rule using an `ssh.hassh` keyword, e.g.:
  ```
  alert ssh any any -> any any (msg:"hassh"; ssh.hassh; content:"a"; sid:1;)
  ```

**Traffic** (attacker is the SSH client, direction = to_server; the symmetric to_client case works identically):

**Segment 1** — SSH banner (so `cli_hdr.flags` becomes `SshStateBannerDone`):
```
53 53 48 2d 32 2e 30 2d 45 76 69 6c 5f 31 2e 30 0d 0a
("SSH-2.0-Evil_1.0\r\n", 18 bytes)
```

**Segment 2** — oversized KEXINIT header + ≥1 body byte in the **same** stream slice (here `remlen = 4`):
```
00 01 00 01    pkt_len     = 0x00010001 = 65537  (be_u32; ssh_parse_record_header only checks >1)
04             padding_len = 4 (unused here)
14             msg_code    = 20 = SSH_MSG_KEXINIT
aa aa aa aa    4 arbitrary body bytes  → rem.len() = 4
```
Total 10 bytes; the full body would be `pkt_len - 2 = 65535` bytes, so `ssh_parse_record()` returns `Incomplete`.

Parser path for segment 2:
- `ssh_parse_record` → `Incomplete` → `ssh_parse_record_header` → `Ok`, `remlen = 4`.
- L277: `record_left = 65537 - 2 - 4 = 65531` (correct).
- L283/285: `Kexinit` + hassh ⇒ `record_left = 65537 - 2 = 65535`.
- L286: `65535 < 65535` is **false** ⇒ `else`: set `LongKexRecord`.
- L300: `return AppLayerResult::ok()`.
- Framework marks the 4 body bytes as consumed; `record_left` is now 4 too large.

**Segment 3** — remaining 65531 body bytes (e.g. all `0x00`) immediately followed by a normal record, e.g. `SSH_MSG_NEWKEYS`:
```
<65531 × 00>
00 00 00 0c 0a 15 00 00 00 00 00 00 00 00 00 00
(pkt_len=12, padding=10, msg_code=21 NEWKEYS, 10 pad bytes)
```

On entry `hdr.record_left = 65535`, so the skip block at L146-172 discards the first 65535 bytes — i.e. the 65531 real remaining body bytes **plus** the first 4 bytes (`00 00 00 0c`) of the NEWKEYS header. Parsing resumes mid-record at bytes `0a 15 00 00 ...`, which `ssh_parse_record_header` reads as `pkt_len = 0x0a150000` (huge) or, depending on remaining length, raises `InvalidRecord`. The NEWKEYS is never recognised, hassh for this direction is never computed, and the parser typically returns `AppLayerResult::err()` and stops inspecting this flow direction.

**Status:** This trigger was fully constructed analytically from source review; it was **not** executed against a live Suricata build (static analysis only per task constraints). No blocker was encountered — the byte sequence above deterministically reaches the buggy code path given hassh is enabled.

## Severity

**LOW** — Detection-bypass / parser desynchronisation, affecting only the attacker-controlled direction.

After the desync the SSH record parser for that direction either mis-frames every subsequent record or sets `SSHEvent::InvalidRecord` and returns `AppLayerResult::err()`, so hassh is never computed and `ssh.hassh` / `ssh.hassh.string` (or the `.server` variants if the attacker is the server) rule keywords never match for that flow; SSH eve-log `hassh` fields stay empty.

There is no crash, no out-of-bounds access, and no memory-safety issue: the over-skip uses safe Rust slicing (L158/170), and `record_left_msg` stays `Undefined` so the Kexinit reassembly arm is not entered with bad state. The legitimate peer's direction is unaffected because it uses a separate `SshHeader` (`ssh.rs:139-143`). The oversized record already raises the `SSHEvent::LongKexRecord` anomaly event, providing a detection signal for defenders.

Exploitation requires non-default configuration (hassh enabled via yaml or via any loaded `ssh.hassh*` rule), and real SSH peers will typically drop a 64 KiB+ KEXINIT record, so this is primarily an IDS-side evasion of SSH client/server fingerprinting — something the attacker could already achieve trivially by sending a different or garbage KEXINIT.

## Suggested Fix

Do not overwrite the correctly-computed `hdr.record_left` from L277 in the oversized (`LongKexRecord`) path. Compute the size check against a local variable and only set `hdr.record_left = head.pkt_len - 2` in the branch that returns `AppLayerResult::incomplete` (where the framework will re-deliver the body bytes anyway). Diff for `rust/src/ssh/ssh.rs`:

```diff
@@ -283,13 +283,14 @@
                                 parser::MessageCode::Kexinit if hassh_is_enabled() => {
                                     // check if buffer is bigger than maximum reassembled packet size
-                                    hdr.record_left = head.pkt_len - 2;
-                                    if hdr.record_left < SSH_MAX_REASSEMBLED_RECORD_LEN as u32 {
+                                    let body_len = head.pkt_len - 2;
+                                    if body_len < SSH_MAX_REASSEMBLED_RECORD_LEN as u32 {
+                                        hdr.record_left = body_len;
                                         // saving type of incomplete kex message
                                         hdr.record_left_msg = parser::MessageCode::Kexinit;
                                         return AppLayerResult::incomplete(
                                             (il - rem.len()) as u32,
-                                            head.pkt_len - 2,
+                                            body_len,
                                         );
                                     } else {
+                                        // keep hdr.record_left = head.pkt_len - 2 - remlen from above
                                         SCLogDebug!("SSH buffer is bigger than maximum reassembled packet size");
                                         self.set_event(SSHEvent::LongKexRecord);
                                     }
                                 }
```

Subtasks 1 (1 open0 closed)

Security #8987: ssh: miscalculation due to oversized KEXINIT causes parser desync (8.0.x backport)AssignedGiuseppe LongoActions
Actions

Also available in: PDF Atom