DF-0965 — twofish_set_key missing key-length validation

Summary

twofish_set_key() at sys/crypto/twofish/twofish.c:425 does not validate its key_len_bits argument. For key_len_bits >= 320 (k_len >= 5) the function performs out-of-bounds writes on size-4 stack arrays (me_key[4]/mo_key[4]), out-of-bounds writes on the struct's s_key[4] (continuing into mk_tab[4*256]), and out-of-bounds reads of the caller's key buffer. The gen_mk_tab switch also silently falls through for unsupported sizes.

Reachability: All in-kernel callers (twofish_cbc_setkey, twofish_xts_setkey in sys/crypto/cryptoapi/cryptoapi.c) validate the key length against {128,192,256}/{256,512} before calling, and cryptoapi_cipher_find() requires probe() to accept the size. So the bug is NOT reachable through the kernel syscall surface. This is a defense-in-depth hardening gap.

Reproduce (function-level harness)

./build.sh && ./run.sh

Expected output (function-level proof, runs as maxx):

[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). ...

Fix-validation (function-level)

./build_fixed.sh    # applies fix.diff to a copy of twofish.c, rebuilds, runs

Expected output (oversized keylen now refused):

[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)

Files

• trigger.c — function-level harness (compiles real twofish.c)
• build.sh / run.sh — build + run the harness
• build_fixed.sh — applies fix.diff to harness source and re-tests
• fix.diff — git-apply-able unified diff against sys/crypto/twofish/twofish.c
• build.log / run.log / fix_run.log — full untrimmed outputs
• env.txt — guest environment
• VERDICT.md — detailed analysis
• manifest.json — artifact catalog

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:

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:

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:

-    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.