Skip to content

core: rtw_ieee80211: guard attr_len underflow in rtw_get_p2p/wfd_attr_content#199

Closed
breakneck-git wants to merge 1 commit into
morrownr:mainfrom
breakneck-git:fix-p2p-wfd-attr-len-underflow
Closed

core: rtw_ieee80211: guard attr_len underflow in rtw_get_p2p/wfd_attr_content#199
breakneck-git wants to merge 1 commit into
morrownr:mainfrom
breakneck-git:fix-p2p-wfd-attr-len-underflow

Conversation

@breakneck-git

Copy link
Copy Markdown
Contributor

Summary

Guards against an integer underflow in rtw_get_p2p_attr_content() and rtw_get_wfd_attr_content() that, on a malformed P2P / Wi-Fi-Display attribute, leads to _rtw_memcpy() writing a near-INT_MAX number of bytes into a kernel buffer.

Same fix submitted as aircrack-ng/rtl8812au#1258.

Root cause

Both functions follow the same pattern:

attr_ptr = rtw_get_p2p_attr(p2p_ie, p2p_ielen, target_attr_id, NULL, &attr_len);
if (attr_ptr && attr_len) {
    if (buf_content)
        _rtw_memcpy(buf_content, attr_ptr + 3, attr_len - 3);   // ← underflow
    ...

attr_len carries the 3-byte attribute header. The parser computes attr_len = attr_data_len + 3 with both operands u16. When attr_data_len >= 0xFFFD (attacker-controlled length field in frame), the addition wraps in u16 to {0, 1, 2}. The parser's bounds check only catches attr_len > p2p_ielen, not the wrap.

When attr_len ∈ {1, 2} reaches the callee, attr_len - 3 underflows in u32 to a near-INT_MAX value, and _rtw_memcpy(buf_content, attr_ptr + 3, ~0xFFFFFFFE) runs — into buf_content which several callers pass as a fixed-size stack buffer (e.g. groupid[38], dev_addr[ETH_ALEN], pwdinfo->p2p_peer_interface_addr[6]).

That's a remote kernel buffer overflow reachable from a crafted P2P / WFD frame on any system using this driver with P2P enabled (default).

Fix

Replace the attr_len truthiness check with attr_len >= 3 (the documented minimum). The new condition is strictly stricter than the old one — when both layers agree the attribute is well-formed, behaviour is unchanged. When the parser returns underflowed attr_len < 3, the callee now returns NULL with *len_content == 0, matching the documented "attribute not found" return path that all callers already handle.

Both rtw_get_p2p_attr_content() and rtw_get_wfd_attr_content() are patched (same pattern).

Notes

  • Defensive minimal fix at the callee. A deeper fix in rtw_get_p2p_attr() / rtw_get_wfd_attr() (validating attr_data_len <= 0xFFFC before + 3) would close the underflow at its source; happy to send a follow-up PR if you prefer defence-in-depth.
  • No runtime test added — trigger requires a crafted 802.11 P2P / WFD action frame which is not part of any existing test harness.

Test environment

  • Realtek 8821AU (USB ID 0bda:0811) on Raspberry Pi 2B, kernel 6.12.47
  • Static analysis only; not runtime-fuzzed
  • Code review verified all 20+ callers of rtw_get_p2p_attr_content handle NULL return correctly

Disclosure

This patch is the result of a code-review pass conducted with the help of an LLM (Claude). The reasoning, exploit chain, and fix were verified by reading the relevant source paths exhaustively before submission. I am not a kernel engineer.

…tent

rtw_get_p2p_attr_content() unconditionally subtracts 3 from attr_len
returned by rtw_get_p2p_attr() and uses the result as a memcpy() length
into buf_content:

    if (attr_ptr && attr_len) {
        if (buf_content)
            _rtw_memcpy(buf_content, attr_ptr + 3, attr_len - 3);
        if (len_content)
            *len_content = attr_len - 3;
        ...

The attr_len value carries the 3-byte P2P attribute header (1 byte ID +
2 bytes length). A malformed frame can yield attr_len < 3 in two ways:

1. The length field in the frame is u16 and rtw_get_p2p_attr() computes
   attr_len = attr_data_len + 3 with both operands being u16. When
   attr_data_len >= 0xFFFD this addition wraps in u16 to {0,1,2}; the
   bounds check in the parser only guards against attr_len being too
   large, not against this underflow.

2. A peer that emits a malformed Length=0 attribute trivially yields
   attr_len = 3 in the parser, which is still safe (== 3 path: copies
   zero bytes). But any path that hands attr_len < 3 directly (or via
   variable types where the wrap happens differently) reaches the same
   subtract.

Once attr_len < 3 reaches the subtract, (attr_len - 3) underflows in
u32 to a near-INT_MAX value. The subsequent _rtw_memcpy() then copies
that many bytes from frame data into buf_content, which several callers
pass as a fixed-size stack buffer:

  - process_p2p_group_negotiation_req: groupid[38], dev_addr[ETH_ALEN]
  - process_p2p_group_negotiation_resp: same
  - rtw_p2p.c:2696,2702 etc: pwdinfo->p2p_peer_interface_addr (6 bytes),
    listen_ch_attr, etc.

That's a kernel stack/heap buffer overflow reachable from a crafted P2P
frame on any system with P2P enabled and a 1x1 dual-band Realtek USB
dongle (CONFIG_P2P=y is the default in this fork).

Fix: require attr_len >= 3 before performing the subtract. The new
condition is strictly stricter than the old one (subset), so the only
behaviour change is that previously-overflowing calls now safely return
NULL with *len_content == 0, matching the documented "attribute not
found" path.

This is a defensive minimal fix; a deeper fix at the parser layer
(validating attr_data_len <= 0xFFFC before adding 3) is also possible
and would be a separate change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant