bcmdhd63.sys — out-of-bounds read / information disclosure in the status-indication path (CWE-125, CWE-200) fixed
KB5073723
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,a5is stored intoStatusIndication.StatusBufferand passed toprint_indicate_statusregardless of the value ofa6. When called with a size of0, a non-NULL but zero-length buffer pointer is handed toNdisMIndicateStatusExand toprint_indicate_status.print_indicate_status(unpatched0x14001C264) never receives the size: its only guard isif (a4 == 0)(atest rbx,rbx/jnzat0x14001C2A8). 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 size0is 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 checksa6 == 0; if so it forcesStatusIndication.StatusBuffer = NULLbeforeNdisMIndicateStatusExand passesNULL(instead ofa5) toprint_indicate_status, which then takes thea4 == 0safe 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 callsosl_debug_mfree, then sets*arg3 = 0and*arg4 = 0. The patched code (sub_140040fc0is 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 callosl_debug_mfree, then set*arg3 = 0and*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 toosl_debug_mfree(9849in the helper vs9886inline) 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:
- Obtain a Handle: Bring up the Broadcom WiFi miniport adapter so the driver processes firmware events.
- 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
0but whose buffer pointer argument (a5) is non-NULL. The exact event depends on firmware and AP/peer behavior. - Reach the sink: In the unpatched driver,
shared_indicate_statusstores the non-NULLa5intoStatusIndication.StatusBufferand forwards it toprint_indicate_statusdespite the zero size, so the downstream consumer may read a zero-length or stale buffer. - Observable Effect: Depending on what
print_indicate_statusdoes 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
- At
shared_indicate_statusentry: - Read
a6(size). Watch for the case where it is0. - Read
a5(buffer pointer). In the vulnerable path it is non-NULL while size is0. - At
print_indicate_statusentry: - Inspect the 4th argument (
r9). In the unpatched binary it equalsa5even when size was0; in the patched binary it reads0x0000000000000000when size was0.
Key Offsets
- Unpatched sink:
0x140012a2cregion —a5forwarded toprint_indicate_statuswithout a size check. - Patched mitigation:
0x14001336e(and [StatusIndication.StatusBuffer], 0) and0x14001338c(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 forsize == 0to prevent OOB reads / info disclosure by forcing the indicated buffer and theprint_indicate_statusargument 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 helpersub_14003f534/sub_140040fc0with 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 is3.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. Addedstrcmp("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_statuschange 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. Thewl_register_ie_updchange 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_statusOOB read requires the driver to emit a status indication with size0and 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_statusconsumes the buffer pointer. - Verification Required: A researcher confirming the OOB read should instrument
shared_indicate_status/print_indicate_statusand capture a live zero-length status indication (e.g., via connect/disconnect or P2P event flows) to observe the pointer forwarded downstream.