1. Overview

  • Unpatched Binary: bcmdhd63_unpatched.sys
  • Patched Binary: bcmdhd63_patched.sys
  • Overall Similarity: 0.9591
  • Diff Statistics: 1138 matched functions (1024 identical, 114 changed), 0 unmatched functions in either direction.

Verdict: This patch adds a defensive guard in the NDIS status-indication path (shared_indicate_status) that avoids passing a zero-length buffer pointer to NdisMIndicateStatusEx and to print_indicate_status, preventing a potential out-of-bounds read / kernel information disclosure (CWE-125). It also refactors the vendor Information Element (IE) OID handler (wl_register_ie_upd) by inlining an existing free-and-NULL cleanup helper; that change is functionally equivalent and is not a memory-safety fix.


2. Vulnerability Summary

Finding 1: Out-of-Bounds Read in shared_indicate_status

  • Severity: Medium
  • Vulnerability Class: Out-of-Bounds Read / Information Disclosure (CWE-125 / CWE-200)
  • Affected Function: shared_indicate_status (Unpatched address: 0x140012a2c; Patched address: 0x1400132e8)
  • Root Cause: The function accepts a status buffer pointer (a5) and its size (a6). In the unpatched code, a5 is stored into StatusIndication.StatusBuffer and passed to print_indicate_status regardless of the value of a6. When called with a size of 0, a non-NULL but zero-length buffer pointer is handed to NdisMIndicateStatusEx and to print_indicate_status. print_indicate_status (unpatched 0x14001C264) never receives the size: its only guard is if (a4 == 0) (a test rbx,rbx / jnz at 0x14001C2A8). For any non-NULL pointer it dispatches on the status code and reads bytes at fixed offsets [a4+4], [a4+6], [a4+8], [a4+9], [a4+0xC], [a4+0x10] and formats them into a log string. So a non-NULL pointer with size 0 is read up to 17 bytes past a zero-length buffer, producing an out-of-bounds read whose bytes reach a formatted debug/log string (kernel information leak). The patch checks a6 == 0; if so it forces StatusIndication.StatusBuffer = NULL before NdisMIndicateStatusEx and passes NULL (instead of a5) to print_indicate_status, which then takes the a4 == 0 safe path. Both builds already carry a stack cookie (__security_cookie / __security_check_cookie); the guard, not the cookie, is the fix.
  • Attacker-Reachable Entry Point: Reachable indirectly via firmware events triggering WiFi connect/disconnect cycles or Wi-Fi Direct state transitions that produce a zero-length status indication.

Finding 2: Vendor IE Cleanup Refactor in wl_register_ie_upd

  • Severity: Informational (not a vulnerability)
  • Change Class: Behavioral / refactoring (no memory-safety impact)
  • Affected Function: wl_register_ie_upd (Unpatched address: 0x140039a94; Patched address: 0x14003b4ec)
  • Description: When a vendor-specific IE update fails, the error-cleanup path frees the IE buffer. The unpatched code performs this by calling the helper sub_14003f534(arg1, arg3, arg4, 0, 0). That helper reads *arg3, and when it is non-NULL calls osl_debug_mfree, then sets *arg3 = 0 and *arg4 = 0. The patched code (sub_140040fc0 is the same helper at its new address) removes the helper call from the error path and inlines the identical sequence: read *arg3, if non-NULL call osl_debug_mfree, then set *arg3 = 0 and *arg4 = 0. Because both the helper and the inlined code NULL the freed pointer and zero the size, no dangling pointer is left in either build; there is no use-after-free. The observable difference is source line metadata passed to osl_debug_mfree (9849 in the helper vs 9886 inline) and the inlining itself.

3. Pseudocode Diff

shared_indicate_status (OOB Read Fix)

// --- UNPATCHED (0x140012a2c) ---
StatusIndication.StatusBufferSize = a6;
StatusIndication.StatusBuffer     = a5;   // stored regardless of size
NdisMIndicateStatusEx(a2, &StatusIndication);

// a5 passed directly. If a6 (size) == 0, this is a zero-length/OOB read downstream.
return print_indicate_status(a1, a3, a4, a5);

// --- PATCHED (0x1400132e8) ---
StatusIndication.StatusBuffer     = a5;
StatusIndication.StatusBufferSize = a6;
if (a6 == 0) {
    DbgPrint("[BRCM]%s: buf size is 0, ptr=0x%x, force StatusBuffer=null\n",
             "shared_indicate_status", a5);
    StatusIndication.StatusBuffer = NULL;   // FIX: do not indicate a stale zero-length buffer
}
NdisMIndicateStatusEx(a2, &StatusIndication);

void* v10 = (a6 != 0) ? a5 : NULL;          // FIX: zero size -> pass NULL downstream
return print_indicate_status(a1, a3, a4, v10, ...);

wl_register_ie_upd (Cleanup Refactor)

// --- UNPATCHED (0x140039a94) ---
if ((unsigned)sub_14003f534(arg1, arg3, arg4, arg5, arg6) != 0)
    return 0xffffffff;
if (arg5 != 0 && arg6 != 0) {
    if ((unsigned)wl_upd_vndr_ies(arg1, arg6, arg5, 1, arg2) != 0) {
        // helper frees *arg3, then sets *arg3 = 0 and *arg4 = 0
        sub_14003f534(arg1, arg3, arg4, 0, 0);
        return 0xffffffff;
    }
}
return 0;

// --- PATCHED (0x14003b4ec) ---
if ((unsigned)sub_140040fc0(arg1, arg3, arg4, arg5, arg6) != 0)  // same helper, new address
    return 0xffffffff;
if (arg5 != 0 && arg6 != 0) {
    if ((unsigned)wl_upd_vndr_ies(arg1, arg6, arg5, 1, arg2) != 0) {
        // inlined equivalent of the helper's free-and-NULL cleanup
        if (*arg3 != 0) {
            osl_debug_mfree(*(*(*arg1 + 8) + 0x10), *arg3, *arg4, 0x269e, "..\\..\\wl\\sys\\wl_oidext.c");
            *arg3 = 0;   // NULL the pointer (as the helper already did)
            *arg4 = 0;   // zero the size (as the helper already did)
        }
        return 0xffffffff;
    }
}
return 0;

4. Assembly Analysis

shared_indicate_status Assembly Diff

The patched binary intercepts the zero-size condition before the downstream calls: it forces the indicated StatusBuffer to NULL and zeroes the r9 register (the 4th argument / buffer pointer) passed to print_indicate_status.

; --- UNPATCHED (0x140012a2c) ---
; No check for a6 == 0
; StatusBuffer = a5 (rsi) stored unconditionally
call    print_indicate_status  ; a5 passed in register regardless of size

; --- PATCHED (0x1400132e8) ---
test    r15d, r15d       ; check a6 (size) == 0
jnz     0x140013373      ; skip if size is non-zero
; ... DbgPrint "buf size is 0" ...
and     [rbp+StatusIndication.StatusBuffer], 0  ; force indicated buffer to NULL
loc_140013373:
call    NdisMIndicateStatusEx_0
test    r15d, r15d       ; check a6 == 0 again
jnz     0x140013391
xor     r9d, r9d         ; r9 = NULL for print_indicate_status
jmp     0x140013394
loc_140013391:
mov     r9, rsi          ; non-zero size -> use buffer
loc_140013394:
call    print_indicate_status

wl_register_ie_upd Assembly Diff

In the unpatched binary the error-cleanup path calls the helper. In the patched binary that call is replaced by the helper's own logic inlined: read the pointer, free it if non-NULL, then zero the pointer and size. Both forms NULL the freed pointer.

; --- UNPATCHED (0x140039a94) ---
; error-cleanup path (0x140039b2a)
call    sub_14003F534    ; helper: mfree(*arg3); *arg3 = 0; *arg4 = 0

; --- PATCHED (0x14003b4ec) ---
; error-cleanup path (0x14003b571)
mov     rdx, [rbx]       ; rdx = *arg3
test    rdx, rdx
jz      short skip_free
; ... setup args for osl_debug_mfree (line 0x269e, "..\\..\\wl\\sys\\wl_oidext.c") ...
call    osl_debug_mfree  ; free the buffer
and     qword [rbx], 0   ; *arg3 = 0   (0x14003b59e)
and     dword [rsi], 0   ; *arg4 = 0   (0x14003b5a2)
skip_free:

5. Trigger Conditions

To exercise the shared_indicate_status out-of-bounds read guard:

  1. Obtain a Handle: Bring up the Broadcom WiFi miniport adapter so the driver processes firmware events.
  2. Induce a zero-length status indication: Drive a WiFi connect/disconnect cycle or a Wi-Fi Direct (P2P) state transition whose status payload size is 0 but whose buffer pointer argument (a5) is non-NULL. The exact event depends on firmware and AP/peer behavior.
  3. Reach the sink: In the unpatched driver, shared_indicate_status stores the non-NULL a5 into StatusIndication.StatusBuffer and forwards it to print_indicate_status despite the zero size, so the downstream consumer may read a zero-length or stale buffer.
  4. Observable Effect: Depending on what print_indicate_status does with the pointer, this can leak adjacent kernel data into logs/telemetry or, under Driver Verifier Special Pool, surface as a pool read violation. The patched driver forces the pointer to NULL when the size is zero.

6. Exploit Primitive & Development Notes

  • Primitive Provided: A conditional, firmware-event-driven out-of-bounds read / kernel information disclosure. It is not directly attacker-controlled from user mode; it depends on the driver producing a zero-length status indication with a non-NULL buffer pointer.
  • Reliability: Low. Triggering requires a specific firmware event sequence and the amount of data exposed depends on print_indicate_status's handling of the buffer pointer.
  • Escalation: Limited. An information leak of this kind could, at best, contribute to defeating KASLR if it exposed kernel pointers, but there is no memory-corruption primitive here and no path to code execution from this finding alone.
  • Mitigations & Bypasses:
  • kASLR: Any pointer leaked through the indicated buffer could help defeat KASLR, but the leak is opportunistic.
  • Driver Verifier: Special Pool will flag an over-read against a zero-length allocation, aiding confirmation.

7. Debugger PoC Playbook

For a researcher with a kernel debugger (KD/WinDbg) attached to the unpatched binary, use these steps to observe the zero-size status-indication path.

Breakpoints

bp bcmdhd63_unpatched!shared_indicate_status "!.echo \"Entered shared_indicate_status\"; r rcx; r r9; r [rsp+28];"
bp bcmdhd63_unpatched!print_indicate_status  "!.echo \"Entered print_indicate_status\"; r rcx; r r9;"

(Note: r9d = a6 size at entry, rsi/stacked arg = a5 buffer pointer, based on the observed prologue.)

What to Inspect at Each Breakpoint

  1. At shared_indicate_status entry:
  2. Read a6 (size). Watch for the case where it is 0.
  3. Read a5 (buffer pointer). In the vulnerable path it is non-NULL while size is 0.
  4. At print_indicate_status entry:
  5. Inspect the 4th argument (r9). In the unpatched binary it equals a5 even when size was 0; in the patched binary it reads 0x0000000000000000 when size was 0.

Key Offsets

  • Unpatched sink: 0x140012a2c region — a5 forwarded to print_indicate_status without a size check.
  • Patched mitigation: 0x14001336e (and [StatusIndication.StatusBuffer], 0) and 0x14001338c (xor r9d, r9d) — both absent in the unpatched build.

Trigger Setup

Drive the adapter through connect/disconnect or Wi-Fi Direct state changes and watch for a status indication whose StatusBufferSize is 0. This is firmware-dependent and may require specific AP or P2P peer behavior.

Expected Observation

When a6 == 0, the unpatched print_indicate_status receives a non-NULL pointer to a zero-length buffer and may read past it. With Special Pool enabled this can surface as a pool read violation; otherwise it may leak adjacent kernel data into the status-print path. In the patched build the pointer is NULL and the read is avoided.


8. Changed Functions — Full Triage

  • shared_indicate_status (Sim: 0.3287): SECURITY RELEVANT. Added a check for size == 0 to prevent OOB reads / info disclosure by forcing the indicated buffer and the print_indicate_status argument to NULL (CWE-125). The stack cookie is present in both builds and is not part of this change.
  • wl_register_ie_upd (Sim: 0.9325): Behavioral. Inlined the vendor-IE error-cleanup free that was previously performed by calling the helper sub_14003f534/sub_140040fc0 with zero allocation arguments. Both forms free and NULL the pointer and zero the size; functionally equivalent, no memory-safety change.
  • wl_pnp_set_power (Sim: 0.7137): Behavioral. Refactored PnP power state transition logic and added device reset (dhd_bus_is_ioready) state tracking.
  • dhd_prot_ioctl (Sim: 0.9633): Behavioral. Added a guard preventing IOCTL dispatch when bus type is 3.
  • wl_cache_M1 (Sim: 0.6207): Behavioral. Feature addition; introduced EAPOL M1 frame caching with a 3000ms timer.
  • wl_oid_phy_build_attrs (Sim: 0.9347): Behavioral. Feature addition; mapped channel 0xa to rate 7 for 802.11ax (Wi-Fi 6) support.
  • wl_set_infra (Sim: 0.9492): Behavioral. Logic update excluding channel 6 from an 802.11n mode check.
  • wl_Rssi2LQ (Sim: 0.8618): Behavioral. Added mapping thresholds for a new PHY type (arg2 == 5) and added a stack cookie.
  • wl_read_reg_param (Sim: 0.2586): Behavioral. Major refactoring of registry parameter parsing loops into standalone structures.
  • wl_iovar_op (Sim: 0.9682): Cosmetic. Added strcmp("dptx") bypass check and adjusted register variable allocations.

(Note: The remaining ~104 changed functions outside this top-10 list are defined in the diff as purely cosmetic struct shifts and register renaming due to inserted logic altering stack frames).


9. Unmatched Functions

There were 0 unmatched functions added or removed in this patch.


10. Confidence & Caveats

  • Confidence: High. The shared_indicate_status change is a clear defensive guard: the unpatched code forwards a non-NULL buffer pointer regardless of a zero size, and the patch forces NULL in that case, both in the decompilation and the disassembly. The wl_register_ie_upd change is confirmed to be a functionally-equivalent inlining of an existing free-and-NULL helper; both builds NULL the freed pointer and zero the size, so no use-after-free is present.
  • Assumptions:
  • The shared_indicate_status OOB read requires the driver to emit a status indication with size 0 and a non-NULL buffer pointer; the exact firmware event sequence that produces this needs to be identified empirically.
  • The severity of the leak depends on how print_indicate_status consumes the buffer pointer.
  • Verification Required: A researcher confirming the OOB read should instrument shared_indicate_status/print_indicate_status and capture a live zero-length status indication (e.g., via connect/disconnect or P2P event flows) to observe the pointer forwarded downstream.