[GH-ISSUE #233] junk.c: <r>/<rc>/<rd> size is parsed but never validated #133

Open
opened 2026-10-13 21:06:52 +00:00 by bsakaguchi · 0 comments
Owner

Originally created by @hoaxisr on GitHub (Sep 8, 2026).
Original GitHub issue: https://github.com/amnezia-vpn/amneziawg-linux-kernel-module/issues/233

The three padding-tag parsers accept any value kstrtoint() can parse, including negative numbers and values close to INT_MAX. Only the return code is checked, never the parsed value:

  • parse_r_tag() in src/junk.c:112
  • parse_rc_tag() in src/junk.c:143
  • parse_rd_tag() in src/junk.c:170
if (!val || 0 > kstrtoint(val, 10, &len))
    return -EINVAL;
...
tag->pkt_size = len;          /* len may be negative */

jp_spec_setup() then sums those sizes into an int and uses the sum as an allocation size, while each individual tag size is used independently as a
copy length and as a modifier length:


pkt_size += tag->pkt_size;                                    /* :290 */
spec->pkt = kzalloc(pkt_size, GFP_KERNEL);                    /* :296 */
memcpy(spec->pkt + spec->pkt_size, tag->pkt, tag->pkt_size);  /* :305 */
mod->buf_len = tag->pkt_size;                                 /* :312 */

Reproduction

ip link add awgt type amneziawg

# control: a single negative tag is harmless — the sum is negative,
# kzalloc() fails and the config is rejected
printf '[Interface]\nPrivateKey = %s\nJc = 4\nJmin = 40\nJmax = 70\nS1 = 30\nS2 = 40\nI1 = <r -1>\n' "$(awg genkey)" > a.conf
awg setconf awgt a.conf
# -> Unable to modify interface: Out of memory   (rc=1, config unchanged)

# the problematic case: mixed with a <b> tag the sum stays small and positive,
# so the config is accepted
printf '[Interface]\nPrivateKey = %s\nJc = 4\nJmin = 40\nJmax = 70\nS1 = 30\nS2 = 40\nI1 = <b 0x0102><r -1>\n' "$(awg genkey)" > b.conf
awg setconf awgt b.conf
# -> rc=0, no error
awg showconf awgt | grep '^I1'
# -> I1 = <b 0x0102><r -1>

Suggested fix

Reject non-sane sizes where they are parsed, in all three parsers:

if (!val || 0 > kstrtoint(val, 10, &len) || len < 0)
    return -EINVAL;

Why this matters outside of a lab

I1–I5 usually reach awg setconf verbatim from a .conf file the user received from someone else, so the value is not necessarily one the operator
chose or reviewed.

Originally created by @hoaxisr on GitHub (Sep 8, 2026). Original GitHub issue: https://github.com/amnezia-vpn/amneziawg-linux-kernel-module/issues/233 The three padding-tag parsers accept any value `kstrtoint()` can parse, including negative numbers and values close to `INT_MAX`. Only the return code is checked, never the parsed value: - `parse_r_tag()` in src/junk.c:112 - `parse_rc_tag()` in src/junk.c:143 - `parse_rd_tag()` in src/junk.c:170 ```c if (!val || 0 > kstrtoint(val, 10, &len)) return -EINVAL; ... tag->pkt_size = len; /* len may be negative */ jp_spec_setup() then sums those sizes into an int and uses the sum as an allocation size, while each individual tag size is used independently as a copy length and as a modifier length: pkt_size += tag->pkt_size; /* :290 */ spec->pkt = kzalloc(pkt_size, GFP_KERNEL); /* :296 */ memcpy(spec->pkt + spec->pkt_size, tag->pkt, tag->pkt_size); /* :305 */ mod->buf_len = tag->pkt_size; /* :312 */ ``` ## Reproduction ```sh ip link add awgt type amneziawg # control: a single negative tag is harmless — the sum is negative, # kzalloc() fails and the config is rejected printf '[Interface]\nPrivateKey = %s\nJc = 4\nJmin = 40\nJmax = 70\nS1 = 30\nS2 = 40\nI1 = <r -1>\n' "$(awg genkey)" > a.conf awg setconf awgt a.conf # -> Unable to modify interface: Out of memory (rc=1, config unchanged) # the problematic case: mixed with a <b> tag the sum stays small and positive, # so the config is accepted printf '[Interface]\nPrivateKey = %s\nJc = 4\nJmin = 40\nJmax = 70\nS1 = 30\nS2 = 40\nI1 = <b 0x0102><r -1>\n' "$(awg genkey)" > b.conf awg setconf awgt b.conf # -> rc=0, no error awg showconf awgt | grep '^I1' # -> I1 = <b 0x0102><r -1> ``` Suggested fix Reject non-sane sizes where they are parsed, in all three parsers: ```c if (!val || 0 > kstrtoint(val, 10, &len) || len < 0) return -EINVAL; ``` Why this matters outside of a lab I1–I5 usually reach awg setconf verbatim from a .conf file the user received from someone else, so the value is not necessarily one the operator chose or reviewed.
Sign in to join this conversation.
No labels
pull-request
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
amnezia-vpn/amneziawg-linux-kernel-module#133
No description provided.