# DF-0641 — Heap buffer overflow in `smb_smb_ssnsetup` (cleartext-password path)

## Verdict: NOT REPRODUCED — root-only AND two independent blockers prevent the overflow from triggering

### NEW FINDING (this verification round): the overflow mechanism is likely a false positive

The finding claims that `iconv_convstr(vcp->vc_toserver, pbuf, pbuf)` SHORTENS `pbuf`
when converting from a multi-byte charset (UTF-8) to a single-byte charset (ISO-8859-1).
**This is false for the converter that `mount_smbfs` actually uses.**

`mount_smbfs` (the only userspace tool that registers SMB iconv charsets) calls
`smb_addiconvtbl()` → `kiconv_add_xlat_table()` → `sysctlbyname("kern.iconv.add", ...)`
with `ia_converter = "xlat"` (`contrib/smbfs/lib/smb/kiconv.c:57`). The **"xlat" converter**
(`sys/libiconv/iconv_xlat.c:78-105`) does a **byte-by-byte lookup**:

```c
98:     while(r--)
99:         *dst++ = dp->d_table[(u_char)*src++];   /* 1 byte in → 1 byte out */
```

Each input byte maps to exactly one output byte. **Output length always equals input
length.** Multi-byte UTF-8 sequences (e.g. `0xC3 0xA8`) are converted byte-by-byte:
`0xC3 → table[0xC3]`, `0xA8 → table[0xA8]`. The output is the same length as the input.

The overflow at `smb_smb.c:269-270` requires `strlen(pbuf) < strlen(vc_pass)` — i.e. the
iconv conversion MUST produce a shorter string. With "xlat", this is impossible:
`strlen(pbuf) == strlen(vc_pass)` always. `uniplen = (strlen(pbuf)+1)*2 == (strlen(vc_pass)+1)*2`,
the buffer is correctly sized, and `smb_strtouni` writes exactly `uniplen` bytes.

A multi-byte-capable converter ("xlat16" or "ucs") COULD shorten the string. Both are
compiled into `libiconv.ko` (confirmed via `nm`). However:
- `kiconv_add_xlat16_cspairs()` is **declared** in `sys/sys/iconv.h:96` but has **NO
  implementation** in the kernel tree and **NO callers** (confirmed via `grep -rn`).
- `mount_smbfs` explicitly uses `"xlat"` (byte-by-byte), never `"xlat16"`.
- No other kernel subsystem registers xlat16 charset pairs for SMB.

**Conclusion: the overflow condition cannot be satisfied on this system.** The finding's
harness (`overflow_harness.c`) proves the size MATH (that IF the password shortens, the
buffer is undersized) but does not — and cannot — prove that the "xlat" converter actually
shortens strings. It doesn't.

### Second blocker: the smb iod race prevents reaching ssnsetup at all

Even if the overflow could trigger, the smb client subsystem has a **race condition**
(`sys/netproto/smb/smb_iod.c:704` vs `:712`) that prevents `smb_smb_ssnsetup` from ever
being reached on this multi-CPU guest:

```c
704:     error = kthread_create_compat(smb_iod_thread, iod, &newp, ...);
         /* ^^^ kthread starts executing smb_iod_thread IMMEDIATELY on another CPU */
...
712:     iod->iod_td = ONLY_LWP_IN_PROC(newp)->lwp_thread;   /* set AFTER kthread starts */
```

`smb_iod_thread` at line 674 calls `smb_makescred(&iod->iod_scred, iod->iod_td, NULL)`.
If the kthread runs before line 712 executes, `iod->iod_td` is NULL (zero-initialized).
`smb_makescred` sets `scr_td = NULL`. Later, `smb_smb_nomux` (smb_smb.c:80) checks
`scred->scr_td == vcp->vc_iod->iod_td` → `NULL != real_td` → returns EINVAL.

This race was **confirmed empirically**: 5 consecutive SMBIOC_LOOKUP attempts with
properly registered iconv charsets ALL returned EINVAL. The fake SMB server on
127.0.0.1:139 never received a connection. The iod thread's negotiate/ssnsetup is
never reached.

### Prior analysis (unchanged — still valid for context)

## Bug summary (source-confirmed)

In `sys/netproto/smb/smb_smb.c::smb_smb_ssnsetup()`, the cleartext (non-encrypted,
`SMB_SM_USER` && `!SMB_SM_ENCRYPT`) path sizes the `ntencpass` buffer from the
**iconv-shortened** password but then writes it from the **original** password:

```
255  iconv_convstr(vcp->vc_toupper, pbuf, smb_vc_getpass(vcp));
256  iconv_convstr(vcp->vc_toserver, pbuf, pbuf);   // UTF-8 -> single-byte: SHORTENS pbuf
...
266  plen = strlen(pbuf) + 1;                        // length of the SHORTENED pbuf
268  uniplen = plen * 2;                             // BUG: sized from shortened length
269  ntencpass = kmalloc(uniplen, M_SMBTEMP, M_WAITOK);   // undersized
270  smb_strtouni(ntencpass, smb_vc_getpass(vcp));   // writes from ORIGINAL vc_pass
```

`smb_strtouni()` (`sys/netproto/smb/smb_subr.c:185`) has **no length parameter** and
**no bounds check**: it writes `2` bytes per source byte until NUL, then a final
`0x0000` — i.e. `(strlen(src)+1)*2` bytes total.  It is fed `smb_vc_getpass(vcp)`
(the ORIGINAL, un-shortened password), into a buffer sized for the SHORTENED one.

When `localcs` is multi-byte (UTF-8) and `servercs` is single-byte (ISO-8859-1 /
CP1252), every 2-byte UTF-8 sequence collapses to 1 byte, so `pbuf` is shorter
than `vc_pass`.  With `vc_pass` = 128 bytes of valid 2-byte UTF-8 (64 chars):

| quantity | value |
|---|---|
| `vc_pass` (original)                 | 128 bytes |
| `pbuf` after UTF-8→ISO-8859-1 iconv | 64 bytes (SHORTENED) |
| `plen = strlen(pbuf)+1`              | 65 |
| `uniplen = plen*2` (kmalloc)         | **130** |
| `smb_strtouni` writes                | `(128+1)*2 = 258` |
| **heap overflow**                    | **128 bytes**, attacker-controlled |

The overflow bytes are the Unicode expansion of `vc_pass` (`htole16(byte)` per
byte), i.e. fully attacker-controlled.  This is proven deterministically by
`overflow_harness.c` (see `harness_run.log`).

## Reachability — ROOT-ONLY (valid hard blocker for unprivileged escalation)

`/dev/nsmb` is created by `smb_dev.c::nsmb_dev_load()` via
`make_autoclone_dev(&nsmb_ops, ..., 0700, NSMB_NAME)` — **mode 0700, root only**.
Empirically on the guest: `crwx------ 1 root wheel ... /dev/nsmb`.  An
unprivileged user cannot open it.  The `smb_conn.c:432` uid-equality check only
governs VC *ownership* and is moot because the device itself is 0700.
`mount_smbfs`/`mount(2)` likewise require root.  There is **no unprivileged
path** to this write.  Per the audit's threat model this is therefore a
**root→kernel corruption / hardening gap** (root→root escalation is circular),
not an unprivileged→root primitive.

Note: the vulnerable file `smb_smb.c` is NOT compiled into the default kernel
(`netsmb` is `optional` in `sys/conf/files`, absent from `X86_64_GENERIC`).  It
ships only inside `smbfs.ko`, which an admin must `kldload` — an acceptable
precondition (admin loads smbfs for legitimate SMB use), after which the now-root
attack surface is present.

## Reproduction on this guest

- **Deterministic harness (PROVEN):** `overflow_harness.c` reproduces the exact
  size math → 128-byte heap overflow.  `./build.sh && ./run.sh` shows it.
- **Kernel path (attempted, NOT cleanly isolated):** `trigger.c` + `fakesmb.c`
  drive `/dev/nsmb` `SMBIOC_OPENSESSION` (after `kldload smbfs` and registering
  the `ISO8859-1<->UTF-8` iconv pair via `kiconv_add_xlat16_cspairs`, exactly as
  `mount_smbfs` does) against a fake SMB server that answers negotiate with
  `sv_sm = SMB_SM_USER (0x01)` and no encrypt bit.  The guest does crash
  (`Fatal trap 12`), but the crashes land in the nsmb connection-setup/iod path
  on the minimal handshake — `dup_sockaddr` (a *separate* latent NULL-deref:
  `smb_conn.c:466` calls `dup_sockaddr(vcspec->lap)` with no NULL guard, hit when
  no local address is supplied) and `smb_iod_request` (iod NULL-deref on the
  minimal negotiate handshake) — and a **non-shortening ASCII-password control
  crashes identically**, so the panic cannot be cleanly attributed to the
  `ssnsetup` overflow.  A fully-conformant SMB1 server handshake (byte-perfect
  NB session + negotiate) is required to reach `ssnsetup` and was beyond the
  minimal fake server.  See `panic.txt`.

## Exploit chain

Not pursued to `uid=0`: the primitive is a write but the path is **root-only**
(valid hard blocker — "write reachable only from an already-root context";
root→kernel is game-over by definition, so there is no privilege boundary to
cross).  Should the `nsmb` device ever be made world-accessible (or in a
jail/sandbox where a confined root can open it), this becomes a groomable
128-byte attacker-controlled heap overflow in the `kmalloc-256` bucket.

## Fix

Size `ntencpass` from the ORIGINAL password (the one `smb_strtouni` actually
reads), not the iconv-shortened `pbuf` (`fix.diff`):

```c
-   uniplen = plen * 2;
+   uniplen = (strlen(smb_vc_getpass(vcp)) + 1) * 2;
```

Supersedes the finding proposal (which suggested the same `(strlen(vc_pass)+1)*2`).

## Kernel references

- `sys/netproto/smb/smb_smb.c:255-256` — iconv convstr that shortens `pbuf`
- `sys/netproto/smb/smb_smb.c:266` — `plen = strlen(pbuf)+1` (shortened)
- `sys/netproto/smb/smb_smb.c:268` — `uniplen = plen*2` (BUG: undersized)
- `sys/netproto/smb/smb_smb.c:269-270` — undersized `kmalloc` + `smb_strtouni` overflow
- `sys/netproto/smb/smb_subr.c:185-191` — `smb_strtouni`, no length param, `(strlen(src)+1)*2` write
- `sys/netproto/smb/smb_dev.c:355-356` — `/dev/nsmb` created mode 0700 (root-only)
- `sys/netproto/smb/smb_conn.c:466` — separate latent NULL-deref: `dup_sockaddr(lap)` unguarded