Skip to content

core: rtw_ieee80211: guard attr_len underflow in rtw_get_wfd_attr_content#63

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_wfd_attr_content#63
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_wfd_attr_content() that, on a malformed Wi-Fi-Display attribute, leads to _rtw_memcpy() writing a near-INT_MAX number of bytes into a kernel stack buffer.

Why only WFD here

This fork's rtw_get_p2p_attr_content() (same file, just above) already uses a bounds-safe pattern — callers pre-populate *len_content with sizeof(buf) and the function clamps the copy to that hint. Whoever did that hardening pass apparently missed rtw_get_wfd_attr_content() right below it; this PR addresses just the missed variant.

The same class of bug was also submitted to:

Root cause

attr_ptr = rtw_get_wfd_attr(wfd_ie, wfd_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 WFD 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 > wfd_ielen, not the wrap.

When attr_len < 3 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.

Fix

Replace attr_len truthiness check with attr_len >= 3 (the documented minimum). 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.

A follow-up to mirror the P2P pattern (caller-supplied bounds via *len_content) here would also be valid — happy to do it as a separate PR if you prefer.

Test environment

  • Realtek 8821AU (USB ID 0bda:0811) on Raspberry Pi 2B, kernel 6.12.47
  • Static analysis only; not runtime-fuzzed (the trigger requires a crafted 802.11 WFD action frame)

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; please flag anything I missed.

…tent

This is the same class of bug as the one already addressed in
rtw_get_p2p_attr_content() in this file: a malformed WFD attribute
whose length field is in the range 0xFFFD..0xFFFF causes the parser's
u16 (attr_data_len + 3) to wrap to {0,1,2}. The bounds check in
rtw_get_wfd_attr() catches "too large", not "wrapped".

When attr_len < 3 reaches the callee, (attr_len - 3) underflows in
u32 to a near-INT_MAX value, and _rtw_memcpy(buf_content, attr_ptr+3,
underflowed) copies that many bytes into buf_content, which several
callers pass as a fixed-size stack buffer.

The neighbouring rtw_get_p2p_attr_content() in the same file already
uses a different bounds-safe pattern (caller pre-populates *len_content
with sizeof(buf), function clamps to that). This WFD variant was
apparently missed in that hardening pass, so this patch applies the
minimal "attr_len >= 3" guard here.

A wider hardening pass on rtw_get_wfd_attr_content() to mirror the
P2P pattern (caller-supplied bounds) would also be valid -- happy to
do that as a follow-up if maintainers prefer.
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