# DF-0965 — twofish_set_key missing key-length validation

## Verdict
**REPRODUCED AT FUNCTION LEVEL — NOT REACHABLE AT RUNTIME via the kernel
syscall surface.** Defense-in-depth hardening gap (latent). The primitive
exists; in-kernel callers protect against it; the fix adds validation in
the function itself.

## Mechanism
`twofish_set_key(ctx, in_key, key_len_bits)` at
`sys/crypto/twofish/twofish.c:425` derives `ctx->k_len = key_len_bits / 64`
(line 433) with **no validation** of the supported values {128,192,256} bits
(k_len ∈ {2,3,4}). When called with `key_len_bits ≥ 320` (k_len ≥ 5), the
loop at lines 435–440:

```c
for(i = 0; i < ctx->k_len; ++i) {
    a = LE32(((const u_int32_t *)in_key)[i + i]);      me_key[i] = a;
    b = LE32(((const u_int32_t *)in_key)[i + i + 1]);  mo_key[i] = b;
    s_key[ctx->k_len - i - 1] = mds_rem(a, b);
}
```

performs three OOB effects:

1. **Stack OOB write** — `me_key[4]`/`mo_key[4]` are size-4 stack arrays
   (declared at line 427). For i ≥ 4 the writes clobber adjacent stack locals
   including the `l_key`/`s_key` pointers (line 428). The follow-on loop at
   lines 442–449 then dereferences these corrupted pointers → SIGBUS/SIGSEGV.
2. **Struct OOB write** — `s_key[k_len-i-1]` for i small writes past `s_key[4]`
   into the `mk_tab[4*256]` array (immediately after `s_key` in
   `struct twofish_ctx`). Attacker-controlled `mds_rem(a, b)` outputs land in
   mk_tab.
3. **Out-of-bounds read** — `((const u_int32_t *)in_key)[i+i(+1)]` for i up to
   k_len-1 reads past the caller's key buffer (the caller must pass at least
   `8*k_len` bytes; passing fewer yields a stack/heap over-read).
4. **Silent mk_tab fallback** — `gen_mk_tab`'s `switch` at line 335 only
   handles `k_len ∈ {2,3,4}`; for other values it falls through leaving
   `mk_tab[]` as the OOB-written garbage. Subsequent `twofish_encrypt` /
   `twofish_decrypt` calls index `mk_tab[4*extract_byte(x,n)]` using this
   corrupted table.

## Reachability analysis (the crucial question)

**Every in-kernel caller validates the key length BEFORE calling
`twofish_set_key`:**

| Caller | Location | Validation |
|---|---|---|
| `twofish_cbc_setkey` | `sys/crypto/cryptoapi/cryptoapi.c:664` | `switch (keylen_in_bytes * 8) { case 128: case 192: case 256: … default: return EINVAL; }` |
| `twofish_xts_setkey` | `sys/crypto/cryptoapi/cryptoapi.c:763` | `switch (keylen_in_bytes * 8) { case 256: case 512: … default: return EINVAL; }` then passes `(keylen_in_bytes/2)*8` (always 128 or 256) |
| `cryptoapi_cipher_find` | `sys/crypto/cryptoapi/cryptoapi.c:1003` | requires `cipher->probe(keysize_in_bits) == 0` before the cipher is even returned; `twofish_cbc_probe`/`twofish_xts_probe` only accept the supported sizes |

These wrappers are the only callers of `twofish_set_key` in the entire
kernel tree (verified: `grep -rn 'twofish_set_key' sys/`). The wrappers are
exposed via `cryptoapi_cipher_setkey` to `dm_target_crypt`
(`sys/dev/disk/dm/crypt/dm_target_crypt.c:306,555`). The dm-crypt target
gets its key from a root-issued `dmsetup` table line — i.e. it requires
root (or `vfs.usermount=1` + a privileged helper) to set the cipher at all,
and even then the key length is parsed as a hex string of fixed length.

**Therefore: an unprivileged user CANNOT reach `twofish_set_key` with an
out-of-range `key_len_bits` through any syscall on the default GENERIC
kernel.** This is a valid hard blocker (dead/unreachable at runtime AND no
syscall harness can exercise it without modifying the validated wrappers).

## Function-level proof
The primitive is real and is proven by `trigger.c`, a userspace harness
that compiles the actual `sys/crypto/twofish/twofish.c` source and invokes
`twofish_set_key` with `key_len_bits=640` (k_len=10). Observed output:

```
[control] key_len_bits=256 (k_len=4): status=0x0 exited 0, k_len=4 (function OK)
[trigger] key_len_bits=640 (k_len=10): status=0x0 exited 0, k_len=10 (function accepted oversized key)
BUG CONFIRMED: ctx->k_len=10 > supported max(4). The
  function silently took the bad size, ran the OOB loop at
  twofish.c:435 (i=0..9), wrote past me_key[4]/mo_key[4]
  (stack) and s_key[4] (struct, into mk_tab), then gen_mk_tab
  at twofish.c:335 fell through (k_len not in {2,3,4}) leaving
  mk_tab as the OOB-written garbage.
```

The function accepted the oversized keylen without complaint; k_len became
10; the OOB writes happened silently. (Some k_len values also crash via
stack-pointer corruption — see `trigger.c` history. The current
heap-allocated harness catches the silent case which is the more
exploitable variant if it were ever reachable.)

## Exploit chain
**Not applicable / blocked by valid hard blocker.** The bug is a memory-
corruption primitive, but the kernel-side reachability is gated by
validated wrappers, so no unprivileged syscall can reach it. There is no
privilege boundary to cross; this is a latent / defense-in-depth gap, not
an exploitable bug on the default kernel.

If a future commit ever:
- adds a caller of `twofish_set_key` that does not validate, or
- weakens `twofish_cbc_setkey` / `twofish_xts_setkey` to forward arbitrary
  keylens (or adds a "raw" setkey path),

the function would become a 4-element stack overflow + struct OOB write
primitive directly triggerable from `dmsetup` (which can be reached from
userspace via `vfs.usermount=1` with a root-created dm-crypt volume).

## Fix
The fix validates `key_len_bits` inside `twofish_set_key` itself, so any
future caller is also protected. See `fix.diff`:

```diff
-    ctx->k_len = key_len_bits / 64;   /* 2, 3 or 4 */
+    switch (key_len_bits) {
+    case 128:
+    case 192:
+    case 256:
+        ctx->k_len = key_len_bits / 64;   /* 2, 3 or 4 */
+        break;
+    default:
+        ctx->k_len = 0;
+        return;
+    }
```

## Fix validation
Validated at the **function level** by applying the same patch to the
harness's `twofish_src.c` and re-running (`build_fixed.sh` → `fix_run.log`):

```
[control] key_len_bits=256 (k_len=4): status=0x0 exited 0, k_len=4 (function OK)
[trigger] key_len_bits=640 (k_len=10): status=0x0 exited 0, k_len=0 (no bug observed)
```

The control (valid 256-bit key) is unchanged, and the trigger (oversized
640-bit) now sets `k_len=0` and returns early instead of running the OOB
loop. `fix_status: not_testable` for kernel runtime (the bug isn't
reachable, so a kernel build is behaviorally a no-op); the function-level
test proves the fix closes the primitive.

## PoC changes
The PoC directory was seeded empty; I authored everything here: `trigger.c`
(function-level harness with control + trigger cases via fork/wait),
`build.sh`/`run.sh` repro scripts, `build_fixed.sh`/`df965_patch.py` for
fix-validation, and `fix.diff`.