DF-0704 — No length validation on received sync datagram

Summary

sys/net/ipfw3_basic/ip_fw3_sync.c:208-218 edge_socket_handler casts m->m_data to (int *), (struct cmd_send_test *), and (struct cmd_send_state *) without checking m->m_len is large enough. A short datagram (even 1 byte with the right type tag) would cause OOB reads in the kprintf paths or — if ipfw_sync_install_state_prt were ever wired up — pass an undersized cmd_send_state struct to a future state-install callback.

How to reproduce

Cannot reproduce on a default kernel — see VERDICT.md and run.log.

The entire ip_fw3_sync.c is dead code: the dispatch pointer ip_fw3_ctl_sync_ptr (sys/net/ipfw3/ip_fw3.c:133) is initialised to NULL and never assigned anywhere in the source tree. The probe in this folder issues all three relevant IP_FW_SYNC_* opcodes via IP_FW_X setsockopt and confirms they all return rc=0 with no effect.

./build.sh
ssh dfbsd 'sysctl net.filters_default_to_accept=1; kldload ipfw3.ko; kldload ipfw3_basic.ko'
ssh dfbsd 'cd /root/poc/DF-0704 && ./run.sh'
# expect: probe prints "All three returned without panic" — code is dead

Preconditions

• root (IP_FW_X requires a raw IP socket, which requires SYSCAP_NONET_RAW).
• AND ip_fw3_ctl_sync_ptr must be non-NULL, which never happens in the default source.

Impact

LATENT — currently zero. Would become a remote-firewall-bypass possibility only after both (a) someone wires ip_fw3_sync_modevent into ipfw3_basic_modevent and (b) the install_state TODO is implemented.

Fix

fix.diff adds per-type m_len validation in ip_fw3_sync_edge_socket_handler (reject short datagrams with a kprintf + continue). Hardening for whenever the code path is hooked up.

DF-0704 — No length validation on received sync datagram (ip_fw3_sync.c)

Verdict: NOT REPRODUCED (LATENT — entire ip_fw3_sync.c is dead code on a default kernel)

Mechanism (as cited)

In ip_fw3_sync_edge_socket_handler (line 207+):

m = sio.sb_mb;
type = (int *)m->m_data;                            /* no m_len check */
if (*type == SYNC_TYPE_SEND_TEST) {
    struct cmd_send_test *cmd;
    cmd = (struct cmd_send_test *)m->m_data;
    kprintf("test received %d\n", cmd->num);        /* reads offset 4-8 */
} else if (*type == SYNC_TYPE_SEND_STATE) {
    struct cmd_send_state *cmd;
    cmd = (struct cmd_send_state *)m->m_data;       /* reads ~40 B */
    if (ipfw_sync_install_state_prt != NULL) {
        (*ipfw_sync_install_state_prt)(cmd);
    }
}

A 1-byte datagram with *type==0 reads cmd->num past the mbuf's populated region; a 1-byte datagram with *type==1 would, if ipfw_sync_install_state_prt were set, pass a cmd_send_state reading ~40 bytes from a 1-byte source. The reviewer labels this LATENT, which is accurate.

Why it doesn't reproduce (the bigger reason)

The entire file is unreachable on a default kernel:

• The IP_FW_SYNC_* opcodes are dispatched by ip_fw3_ctl() at sys/net/ipfw3/ip_fw3.c:1113-1128 only if ip_fw3_ctl_sync_ptr != NULL.
• That pointer is initialised to NULL (line 133) and NEVER assigned anywhere in the tree: $ grep -rn "ip_fw3_ctl_sync_ptr =" sys/ (no hits)
• The companion ipfw_sync_send_state_prt global is similarly never assigned: it would be set by ip_fw3_sync_modevent(MOD_LOAD) (ip_fw3_sync.c:470-475), but ip_fw3_sync_modevent is never called by anyone — the ipfw3_basic module hook (ip_fw3_basic.c:657-658) invokes only ip_fw3_state_modevent and ip_fw3_table_modevent, skipping sync entirely.

Empirical confirmation (run.log): on the default kernel with net.filters_default_to_accept=1 (so SSH survives ipfw3's default-deny), after kldload ipfw3.ko and kldload ipfw3_basic.ko, issuing the three "panic-trigger" opcodes via IP_FW_X:

[*] invoking IP_FW_SYNC_EDGE_START   (DF-0705 panic 1)
[fire opcode=85 plen=0] setsockopt rc=0 errno=0
[*] invoking IP_FW_SYNC_CENTRE_CONF  (DF-0705 panic 3, count=-1)
[fire opcode=89 plen=4] setsockopt rc=0 errno=0
[*] invoking IP_FW_SYNC_SHOW_CONF    (DF-0705 panic 3 read-back)
[fire opcode=82 plen=64] setsockopt rc=0 errno=0
[+] All three returned without panic.

All three opcodes silently fall through (the if (sync_ptr != NULL) check fails), so the buggy functions are never invoked.

Fix

Even though the code is currently dead, it would become live the moment someone wires up ip_fw3_sync_modevent into ipfw3_basic_modevent (one-line addition). The fix.diff adds per-type length validation in ip_fw3_sync_edge_socket_handler so that when the code is hooked up, a short datagram is logged-and-skipped instead of reading past its end:

m = sio.sb_mb;
if (m == NULL || m->m_len < sizeof(int)) {
    kprintf("ipfw3sync: ignore short datagram\n");
    continue;
}
type = (int *)m->m_data;
if (*type == SYNC_TYPE_SEND_TEST) {
    if (m->m_len < sizeof(struct cmd_send_test)) {
        kprintf("ipfw3sync: short SEND_TEST\n");
        continue;
    }
    ...

(Matches the LATENT (dead code) label in the finding's summary; the fix is hardening for when the TODO install_state is implemented.)

Files

• ipfw3_sync_probe.c — reachability probe (issues all 3 opcodes, observes fall-through)
• run.log — probe output proving dead code
• fix.diff — per-type length validation
• VERDICT.md — this analysis