[PR #189] [CLOSED] fix: wrong cookie-reply size constant, UB shift in bogus_endpoints #227

New issue

Closed

opened 2026-10-13 21:07:48 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-13 21:07:48 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/amnezia-vpn/amneziawg-linux-kernel-module/pull/189
Author: @pattyboi
Created: 7/20/2026
Status: ❌ Closed

Base: master ← Head: fix/cookie-reply-size-and-bogus-endpoints-shift

📝 Commits (1)

50f67d9 fix: wrong constant for cookie-reply length, UB shift in bogus_endpoints

📊 Changes

2 files changed (+8 additions, -6 deletions)

View changed files

📝 src/netlink.c (+7 -5)
📝 src/receive.c (+1 -1)

📄 Description

Two small, independent correctness fixes found while auditing the
receive path and the (currently unused-by-default) bogus_endpoints
netlink feature:

receive.c: prepare_awg_message()'s handshake-cookie branch returned
MESSAGE_HANDSHAKE_COOKIE (the message-TYPE enum value, 3) instead of
MESSAGE_COOKIE_REPLY_SIZE (sizeof(struct message_handshake_cookie),
~60 bytes) -- a copy-paste mistake, since its three sibling branches
(init/response/transport) correctly return their *_SIZE constant.
The caller uses this return value as the length passed to
pskb_may_pull(), so a wrong value here weakens that check for
cookie-reply packets specifically.

Not currently exploitable: prepare_awg_message() unconditionally
skb_linearize()s any nonlinear skb before any type branch runs, and
a linear skb already has its full length contiguous by definition,
so the weakened pskb_may_pull() call downstream is already trivially
satisfied regardless of the value passed to it. Fixed anyway since
it's a clear bug and matches its siblings' pattern.
netlink.c: generate_ipv4_address_with_prefix() computed
'1U << (32 - prefix_len)', which is undefined behavior when
prefix_len == 0 (shift by the full width of the type). This is part
of the bogus_endpoints feature (obfuscates peer endpoints in 'wg
show' output within a configurable prefix), gated behind a
module_param that defaults to disabled -- but it is a real,
root-reachable code path once enabled with a /0 prefix, not
unreachable.

Fixed by computing the mask in 64 bits (1ULL << ...) before
narrowing back to u32, so the shift amount (0-32) is always within
the wider type's width -- one expression handles both the
prefix_len == 0 and == 32 edge cases without branching. Verified
the resulting suffix_mask against every multiple-of-8 prefix_len
from 0 to 32 (0xffffffff, 0x00ffffff, 0x0000ffff, 0x000000ff, 0).

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/amnezia-vpn/amneziawg-linux-kernel-module/pull/189 **Author:** [@pattyboi](https://github.com/pattyboi) **Created:** 7/20/2026 **Status:** ❌ Closed **Base:** `master` ← **Head:** `fix/cookie-reply-size-and-bogus-endpoints-shift` --- ### 📝 Commits (1) - [`50f67d9`](https://github.com/amnezia-vpn/amneziawg-linux-kernel-module/commit/50f67d91c886cfeecf97b1a445ea55e1c06ffd5f) fix: wrong constant for cookie-reply length, UB shift in bogus_endpoints ### 📊 Changes **2 files changed** (+8 additions, -6 deletions) <details> <summary>View changed files</summary> 📝 `src/netlink.c` (+7 -5) 📝 `src/receive.c` (+1 -1) </details> ### 📄 Description Two small, independent correctness fixes found while auditing the receive path and the (currently unused-by-default) bogus_endpoints netlink feature: 1. receive.c: prepare_awg_message()'s handshake-cookie branch returned MESSAGE_HANDSHAKE_COOKIE (the message-TYPE enum value, 3) instead of MESSAGE_COOKIE_REPLY_SIZE (sizeof(struct message_handshake_cookie), ~60 bytes) -- a copy-paste mistake, since its three sibling branches (init/response/transport) correctly return their *_SIZE constant. The caller uses this return value as the length passed to pskb_may_pull(), so a wrong value here weakens that check for cookie-reply packets specifically. Not currently exploitable: prepare_awg_message() unconditionally skb_linearize()s any nonlinear skb before any type branch runs, and a linear skb already has its full length contiguous by definition, so the weakened pskb_may_pull() call downstream is already trivially satisfied regardless of the value passed to it. Fixed anyway since it's a clear bug and matches its siblings' pattern. 2. netlink.c: generate_ipv4_address_with_prefix() computed '1U << (32 - prefix_len)', which is undefined behavior when prefix_len == 0 (shift by the full width of the type). This is part of the bogus_endpoints feature (obfuscates peer endpoints in 'wg show' output within a configurable prefix), gated behind a module_param that defaults to disabled -- but it is a real, root-reachable code path once enabled with a /0 prefix, not unreachable. Fixed by computing the mask in 64 bits (1ULL << ...) before narrowing back to u32, so the shift amount (0-32) is always within the wider type's width -- one expression handles both the prefix_len == 0 and == 32 edge cases without branching. Verified the resulting suffix_mask against every multiple-of-8 prefix_len from 0 to 32 (0xffffffff, 0x00ffffff, 0x0000ffff, 0x000000ff, 0). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-13 21:07:48 +00:00