odhcp6c: cherry pick patches from main

This fixes multiple bugs, some might be security relevant.

Link: https://github.com/openwrt/openwrt/pull/24258
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
This commit is contained in:
Hauke Mehrtens
2026-07-21 02:46:29 +02:00
parent 67c5b71d77
commit b9ad54db42
14 changed files with 804 additions and 1 deletions
+1 -1
View File
@@ -8,7 +8,7 @@
include $(TOPDIR)/rules.mk
PKG_NAME:=odhcp6c
PKG_RELEASE:=1
PKG_RELEASE:=2
PKG_SOURCE_PROTO:=git
PKG_SOURCE_URL=$(PROJECT_GIT)/project/odhcp6c.git
@@ -0,0 +1,37 @@
From 3d812c338eb93d897717627ebb1b7beb01646e6e Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 11 Jul 2026 23:29:19 +0200
Subject: dhcpv6: fix out-of-bounds end pointer when parsing IA in Advertise
dhcpv6_handle_advert passes 'odata + olen + sizeof(*ia_hdr)' as the end
pointer to dhcpv6_parse_ia. odata points to the start of the IA
option's data (just past the 4-byte option header) and olen is the
data length, so the valid sub-option range ends at 'odata + olen'.
The extra '+ sizeof(*ia_hdr)' (16 bytes) extends the range 16 bytes
past the IA option boundary, allowing dhcpv6_parse_ia's sub-option
iterator to read into adjacent option bytes (or past the receive
buffer when the IA is the last option) and parse them as bogus IA
sub-options, polluting STATE_IA_NA / STATE_IA_PD with garbage.
The sibling call site in dhcpv6_handle_reply already passes the
correct 'odata + olen'. Match that here.
Assisted-by: Claude:claude-opus-4-7
Link: https://github.com/openwrt/odhcp6c/pull/160
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit b6f0c70f5fc26632d8a6c748efec5aa335aa4fde)
---
src/dhcpv6.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/src/dhcpv6.c
+++ b/src/dhcpv6.c
@@ -959,7 +959,7 @@ static int dhcpv6_handle_advert(enum dhc
(otype == DHCPV6_OPT_IA_NA && na_mode != IA_MODE_NONE)) &&
olen > -4 + sizeof(struct dhcpv6_ia_hdr)) {
struct dhcpv6_ia_hdr *ia_hdr = (void*)(&odata[-4]);
- dhcpv6_parse_ia(ia_hdr, odata + olen + sizeof(*ia_hdr));
+ dhcpv6_parse_ia(ia_hdr, odata + olen);
}
if (otype == DHCPV6_OPT_SERVERID && olen <= 130) {
@@ -0,0 +1,49 @@
From ee3b760cb42935f4b6578c0481f75b793eadd0f7 Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 11 Jul 2026 23:31:09 +0200
Subject: script: avoid kill(0) when SIGCHLD races script_call
script_call() does
if (running) {
...
kill(running, SIGTERM);
}
where 'running' is a volatile pid_t cleared to 0 by the SIGCHLD
handler when the previous script child exits. The 'if (running)'
gate and the 'kill(running, ...)' that follows it are independent
reads of the volatile variable. If SIGCHLD fires for the previous
child between these two reads, the handler clears 'running' to 0
and kill() is called with pid == 0, which delivers SIGTERM to every
process in the same process group as odhcp6c, including odhcp6c
itself (and any peer/parent that shares the pgrp).
Snapshot the pid into a local before checking it and using it, so the
kill() target is the value we actually tested.
Assisted-by: Claude:claude-opus-4-7
Link: https://github.com/openwrt/odhcp6c/pull/160
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit 2521bf732b9f643ce36c5ac3b314b17c034a0f50)
---
src/script.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
--- a/src/script.c
+++ b/src/script.c
@@ -402,10 +402,11 @@ void script_call(const char *status, int
time_t now = odhcp6c_get_milli_time() / 1000;
bool running_script = false;
- if (running) {
+ pid_t prev = running;
+ if (prev > 0) {
time_t diff = now - started;
- kill(running, SIGTERM);
+ kill(prev, SIGTERM);
if (diff > delay)
delay -= diff;
@@ -0,0 +1,52 @@
From 240ca48bb8f783177b86a21425c5b1b5bab2b7ae Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 23 May 2026 14:15:36 +0200
Subject: odhcp6c: propagate allocation failure from insert_state
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
odhcp6c_insert_state always returned 0, even when odhcp6c_resize_state
failed (either because the state would exceed the 1024-byte cap or
because realloc() returned NULL). The callers act on the return value:
* config_add_requested_options() logs and aborts when nonzero, but
silently accepts the lost option when zero — so a failed insert
becomes "succeeded but not actually added".
* dhcpv6_add_server_cand() frees the candidate's IA_NA / IA_PD only
when nonzero, so an insert failure both loses the candidate and
leaks its IA buffers.
Return -1 when odhcp6c_resize_state() returns NULL, matching the
documented contract of the function and the behaviour of
odhcp6c_add_state().
Assisted-by: Claude:claude-opus-4-7
Link: https://github.com/openwrt/odhcp6c/pull/160
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit 49d9c0112bb8cd9760cc13495caa5009b61cf2ad)
---
src/odhcp6c.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
--- a/src/odhcp6c.c
+++ b/src/odhcp6c.c
@@ -735,12 +735,13 @@ int odhcp6c_insert_state(enum odhcp6c_st
uint8_t *n = odhcp6c_resize_state(state, len);
- if (n) {
- uint8_t *sdata = state_data[state];
+ if (!n)
+ return -1;
- memmove(sdata + offset + len, sdata + offset, len_after);
- memcpy(sdata + offset, data, len);
- }
+ uint8_t *sdata = state_data[state];
+
+ memmove(sdata + offset + len, sdata + offset, len_after);
+ memcpy(sdata + offset, data, len);
return 0;
}
@@ -0,0 +1,36 @@
From 5495a8b3f20df00866946645b01869598d3ddd74 Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 11 Jul 2026 23:32:40 +0200
Subject: dhcpv6: ensure hostname buffer is NUL-terminated for dn_comp
POSIX permits gethostname() to leave the destination buffer
non-NUL-terminated when the system hostname is at least as long as the
buffer (the trailing NUL is dropped during truncation). Linux glibc
exhibits this behaviour. The buffer is passed directly to dn_comp(),
which scans for the terminator and would otherwise read past
fqdn_buf[].
Pass one fewer byte to gethostname() and explicitly NUL-terminate the
last byte so dn_comp() always sees a properly bounded C string.
Assisted-by: Claude:claude-opus-4-7
Link: https://github.com/openwrt/odhcp6c/pull/160
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit 4bd976fd60c0e2baf00c92991d30507ece2b9075)
---
src/dhcpv6.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/src/dhcpv6.c
+++ b/src/dhcpv6.c
@@ -341,7 +341,8 @@ static void dhcpv6_send(enum dhcpv6_msg
{
// Build FQDN
char fqdn_buf[256];
- gethostname(fqdn_buf, sizeof(fqdn_buf));
+ gethostname(fqdn_buf, sizeof(fqdn_buf) - 1);
+ fqdn_buf[sizeof(fqdn_buf) - 1] = '\0';
struct {
uint16_t type;
uint16_t len;
@@ -0,0 +1,121 @@
From 43f9c5e03c6bf89ef93834c8de57fe6985f88b64 Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 23 May 2026 14:19:39 +0200
Subject: script: handle allocation failures in env helpers
ipv6_to_env(), fqdn_to_env(), bin_to_env(), entry_to_env(),
search_to_env(), int_to_env() and the inline PASSTHRU=... assembler
all called realloc(NULL, ...) / malloc() without checking the result,
then immediately wrote into the (possibly NULL) buffer with memcpy(),
snprintf() or script_hexlify(). Under memory pressure this dereferences
NULL inside the forked script-helper child and crashes it before
putenv() / execv(), so the state script does not run for that
state-change.
Check each allocation and skip the corresponding environment variable
when it fails. While at it, replace the cosmetic realloc(NULL, ...) idiom
with plain malloc() to make intent clearer.
Assisted-by: Claude:claude-opus-4-7
Link: https://github.com/openwrt/odhcp6c/pull/160
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit 01130f80338a250f94ae0df786306c11662c88fb)
---
src/script.c | 42 ++++++++++++++++++++++++++++++++----------
1 file changed, 32 insertions(+), 10 deletions(-)
--- a/src/script.c
+++ b/src/script.c
@@ -98,7 +98,10 @@ static void ipv6_to_env(const char *name
const struct in6_addr *addr, size_t cnt)
{
size_t buf_len = strlen(name);
- char *buf = realloc(NULL, cnt * INET6_ADDRSTRLEN + buf_len + 2);
+ char *buf = malloc(cnt * INET6_ADDRSTRLEN + buf_len + 2);
+
+ if (!buf)
+ return;
memcpy(buf, name, buf_len);
buf[buf_len++] = '=';
@@ -121,7 +124,10 @@ static void fqdn_to_env(const char *name
size_t buf_len = strlen(name);
size_t buf_size = len + buf_len + 2;
const uint8_t *fqdn_end = fqdn + len;
- char *buf = realloc(NULL, len + buf_len + 2);
+ char *buf = malloc(buf_size);
+
+ if (!buf)
+ return;
memcpy(buf, name, buf_len);
buf[buf_len++] = '=';
@@ -148,9 +154,12 @@ static void bin_to_env(uint8_t *opts, si
uint16_t otype, olen;
dhcpv6_for_each_option(opts, oend, otype, olen, odata) {
- char *buf = realloc(NULL, 14 + (olen * 2));
+ char *buf = malloc(14 + (olen * 2));
size_t buf_len = 0;
+ if (!buf)
+ continue;
+
snprintf(buf, 14, "OPTION_%hu=", otype);
buf_len += strlen(buf);
@@ -173,7 +182,10 @@ static void entry_to_env(const char *nam
// Worst case: ENTRY_PREFIX with iaid != 1 and exclusion
const size_t max_entry_len = (INET6_ADDRSTRLEN-1 + 5 + 22 + 15 + 10 +
INET6_ADDRSTRLEN-1 + 11 + 1);
- char *buf = realloc(NULL, buf_len + 2 + (len / sizeof(*e)) * max_entry_len);
+ char *buf = malloc(buf_len + 2 + (len / sizeof(*e)) * max_entry_len);
+
+ if (!buf)
+ return;
memcpy(buf, name, buf_len);
buf[buf_len++] = '=';
@@ -238,8 +250,13 @@ static void entry_to_env(const char *nam
static void search_to_env(const char *name, const uint8_t *start, size_t len)
{
size_t buf_len = strlen(name);
- char *buf = realloc(NULL, buf_len + 2 + len);
- char *c = mempcpy(buf, name, buf_len);
+ char *buf = malloc(buf_len + 2 + len);
+ char *c;
+
+ if (!buf)
+ return;
+
+ c = mempcpy(buf, name, buf_len);
*c++ = '=';
for (struct odhcp6c_entry *e = (struct odhcp6c_entry*)start;
@@ -262,7 +279,10 @@ static void search_to_env(const char *na
static void int_to_env(const char *name, int value)
{
size_t len = 13 + strlen(name);
- char *buf = realloc(NULL, len);
+ char *buf = malloc(len);
+
+ if (!buf)
+ return;
snprintf(buf, len, "%s=%d", name, value);
putenv(buf);
@@ -495,9 +515,11 @@ void script_call(const char *status, int
int_to_env("RA_RETRANSMIT", ra_get_retransmit());
char *buf = malloc(10 + passthru_len * 2);
- strncpy(buf, "PASSTHRU=", 10);
- script_hexlify(&buf[9], passthru, passthru_len);
- putenv(buf);
+ if (buf) {
+ strncpy(buf, "PASSTHRU=", 10);
+ script_hexlify(&buf[9], passthru, passthru_len);
+ putenv(buf);
+ }
execv(argv[0], argv);
_exit(128);
@@ -0,0 +1,52 @@
From 2b2aeabbb9550d9617878f32b6ed1ed2a38e33ad Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 23 May 2026 14:20:53 +0200
Subject: odhcp6c: refuse to follow symlinks when writing pidfile
odhcp6c is generally run as root and writes its pidfile with
fopen(pidfile, "w"), which calls open(O_WRONLY | O_CREAT | O_TRUNC)
underneath. open() follows symlinks by default, so if an attacker (or
a broken init script) can place a symlink at the pidfile path before
odhcp6c starts, the symlink target gets truncated and overwritten with
a single line containing the daemon's PID. With /var/run sometimes
shared between services this gives a primitive for trashing arbitrary
root-owned files, plus the file is created with the symlink target's
ownership instead of root:root.
Use open() with O_NOFOLLOW | O_CLOEXEC explicitly and fdopen() the
result; the file is still truncated and replaced when it is a regular
file, but a symlink in the path causes open() to fail with ELOOP and
we simply skip pidfile creation rather than clobbering the target.
Assisted-by: Claude:claude-opus-4-7
Link: https://github.com/openwrt/odhcp6c/pull/160
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit 0a19052dc9fb73530adeff9b61edb3a35f15bc2a)
---
src/odhcp6c.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
--- a/src/odhcp6c.c
+++ b/src/odhcp6c.c
@@ -454,10 +454,17 @@ int main(_unused int argc, char* const a
pidfile = (char*)buf;
}
- FILE *fp = fopen(pidfile, "w");
- if (fp) {
- fprintf(fp, "%i\n", getpid());
- fclose(fp);
+ int pidfd = open(pidfile,
+ O_WRONLY | O_CREAT | O_TRUNC | O_NOFOLLOW | O_CLOEXEC,
+ 0644);
+ if (pidfd >= 0) {
+ FILE *fp = fdopen(pidfd, "w");
+ if (fp) {
+ fprintf(fp, "%i\n", getpid());
+ fclose(fp);
+ } else {
+ close(pidfd);
+ }
}
}
@@ -0,0 +1,54 @@
From 760b53c7400c2b60cf48723509fc123843ad07f4 Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 23 May 2026 14:21:37 +0200
Subject: odhcp6c: skip malformed /proc/net/if_inet6 entries
The validation loop in odhcp6c_addr_in_scope() walked addr_buf
checking each character with isxdigit() / !isupper(), but discarded
the result: 'i' was a local that went out of scope at the end of the
loop, and execution fell straight into the decode loop regardless. If
the kernel ever produced unexpected characters (or someone bind-mounts
a non-procfs over /proc/net/if_inet6), index(hex, c) would return NULL
and the subsequent 'NULL - hex' subtraction is undefined behaviour;
the address bytes were then derived from arbitrary pointer arithmetic.
Track the validation outcome explicitly, also reject inputs whose
length isn't 32 hex characters (one IPv6 address worth), and continue
to the next /proc line on any failure so the decode loop only sees
well-formed inputs.
Assisted-by: Claude:claude-opus-4-7
Link: https://github.com/openwrt/odhcp6c/pull/160
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit d6c2fbdc255c143afc4d2c670f602c9cc9308869)
---
src/odhcp6c.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
--- a/src/odhcp6c.c
+++ b/src/odhcp6c.c
@@ -940,13 +940,20 @@ bool odhcp6c_addr_in_scope(const struct
(flags & (IFA_F_DADFAILED | IFA_F_TENTATIVE | IFA_F_DEPRECATED)))
continue;
- for (i = 0; i < strlen(addr_buf); i++) {
- if (!isxdigit(addr_buf[i]) || isupper(addr_buf[i]))
- break;
+ size_t addr_len = strlen(addr_buf);
+ bool valid = (addr_len == 2 * sizeof(inet6_addr.s6_addr));
+
+ for (i = 0; valid && i < addr_len; i++) {
+ if (!isxdigit((unsigned char)addr_buf[i]) ||
+ isupper((unsigned char)addr_buf[i]))
+ valid = false;
}
+ if (!valid)
+ continue;
+
memset(&inet6_addr, 0, sizeof(inet6_addr));
- for (i = 0; i < (strlen(addr_buf) / 2); i++) {
+ for (i = 0; i < (addr_len / 2); i++) {
unsigned char byte;
static const char hex[] = "0123456789abcdef";
byte = ((index(hex, addr_buf[i * 2]) - hex) << 4) |
@@ -0,0 +1,50 @@
From a04d72ac6c809cc9f189b635839ddbb9a48e44a3 Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 11 Jul 2026 23:34:04 +0200
Subject: script: handle fork() failure when launching state script
If fork() returns -1 (e.g. under EAGAIN / ENOMEM), neither the parent
nor the child branch runs. Control falls through to the bottom of
script_call() while 'running' still holds the previous (just-killed)
child's pid and 'started' still holds the previous start time. The
next call sees running != 0, fires kill() at a process that no longer
exists, and accumulates a misleading 'diff = now - started' against
the older start time. No diagnostic is logged either.
Log the failure, clear 'running' so subsequent calls do not chase a
ghost pid, and return without going through the (parent-only)
post-fork bookkeeping.
Assisted-by: Claude:claude-opus-4-7
Link: https://github.com/openwrt/odhcp6c/pull/160
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit c938c168cbef5642cab42389d96c340e40f4f8ff)
[use syslog() instead of the error() log helper, which is not present in 24.10]
---
src/script.c | 7 +++++++
1 file changed, 7 insertions(+)
--- a/src/script.c
+++ b/src/script.c
@@ -14,6 +14,7 @@
*/
#include <stdio.h>
+#include <errno.h>
#include <netdb.h>
#include <resolv.h>
#include <stdlib.h>
@@ -441,6 +442,12 @@ void script_call(const char *status, int
pid_t pid = fork();
+ if (pid < 0) {
+ syslog(LOG_ERR, "Failed to fork script handler: %s", strerror(errno));
+ running = 0;
+ return;
+ }
+
if (pid > 0) {
running = pid;
started = now;
@@ -0,0 +1,49 @@
From 49f251805dd5bce1964d4d6549ff1178ae8df2a5 Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 30 May 2026 23:17:47 +0200
Subject: odhcp6c: bound the address length when parsing the -P argument
The -P <[pfx/]len> handler copied everything before the '/' into the
fixed 134-byte stack buffer "buf" with
strncpy((char *)buf, optarg, optpos - optarg);
buf[optpos - optarg] = '\0';
without checking that the address portion fits. An argument whose
address part is 134 bytes or longer overflows buf on the stack (the
trailing NUL store is out of bounds as well).
Reject any address portion that does not fit in buf before copying.
inet_pton() would reject such an over-long string anyway, so no valid
invocation changes behaviour.
Assisted-by: Claude:claude-opus-4-8
Link: https://github.com/openwrt/odhcp6c/pull/160
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit e9a9e9d45f38bee5d761de0421ca52163efa28d7)
---
src/odhcp6c.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
--- a/src/odhcp6c.c
+++ b/src/odhcp6c.c
@@ -245,8 +245,17 @@ int main(_unused int argc, char* const a
optpos = strchr(optarg, '/');
if (optpos) {
- strncpy((char *)buf, optarg, optpos - optarg);
- buf[optpos - optarg] = '\0';
+ size_t addr_len = optpos - optarg;
+
+ /* Leave room for the terminating NUL; reject anything
+ * that cannot be a valid IPv6 literal instead of
+ * overflowing buf. */
+ if (addr_len >= sizeof(buf)) {
+ syslog(LOG_ERR, "invalid argument: '%s'", optarg);
+ return 1;
+ }
+ strncpy((char *)buf, optarg, addr_len);
+ buf[addr_len] = '\0';
if (inet_pton(AF_INET6, (char *)buf, &prefix.addr) <= 0) {
syslog(LOG_ERR, "invalid argument: '%s'", optarg);
return 1;
@@ -0,0 +1,43 @@
From 7d71729e4da366d1c19c205487e4decf6d646dde Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 11 Jul 2026 23:36:10 +0200
Subject: dhcpv6: require known SERVERID when validating Reconfigure
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
dhcpv6_response_is_valid() previously set serverid_ok = true whenever
STATE_SERVER_ID was empty, on the assumption that an empty stored
server-id only happens for Solicit/Advertise where the client has not
yet bound to a server. The same code path is taken for Reconfigure
messages (req_msg_type == DHCPV6_MSG_UNKNOWN), so a Reconfigure
message that arrived while STATE_SERVER_ID was somehow empty (state
corruption, an early Reconfigure delivered before binding completes,
or a server-side bug) would be accepted without server identification.
RFC 8415 §18.2.11 requires the SERVERID in a Reconfigure to match the
server the client received the lease from. Only set serverid_ok = true
on an empty stored server-id when we are not validating a Reconfigure;
for DHCPV6_MSG_UNKNOWN we leave it false so the response is dropped.
Assisted-by: Claude:claude-opus-4-7
Link: https://github.com/openwrt/odhcp6c/pull/160
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit 0a4e51db30d5a13175cdda7830a5fcb882e88659)
[the message-type parameter is named 'type' in 24.10, not 'req_msg_type']
---
src/dhcpv6.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/src/dhcpv6.c
+++ b/src/dhcpv6.c
@@ -819,7 +819,7 @@ static bool dhcpv6_response_is_valid(con
if (server_id_len)
serverid_ok = (olen + 4U == server_id_len) && !memcmp(
&odata[-4], server_id, server_id_len);
- else
+ else if (type != DHCPV6_MSG_UNKNOWN)
serverid_ok = true;
} else if (otype == DHCPV6_OPT_AUTH && olen == -4 +
sizeof(struct dhcpv6_auth_reconfigure)) {
@@ -0,0 +1,49 @@
From 1374cbad07c63db19bba0f371c1a2f36167381dd Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 23 May 2026 14:46:55 +0200
Subject: odhcp6c: do not treat DHCPv6 option type 0 as end-of-list
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
dhcpv6_for_each_option was structured as a chain of '&&' expressions
where each link both assigned a loop variable (otype, odata, olen)
and gated iteration on the assigned value being truthy. The middle
link, '((otype) = _o[0] << 8 | _o[1])', made iteration stop on
otype == 0.
DHCPv6 has no end-of-list sentinel; option code 0 is just reserved
(RFC 8415 §21, IANA registry). A server (broken or hostile) that emits
an option with code 0 anywhere in the response would silently truncate
the client's view of the option list, masking everything that follows
— including STATUS, IA_NA / IA_PD, AUTH, etc.
Switch the assignments inside the for-loop condition to use the comma
operator so all three (otype, odata, olen) are unconditionally
written, and gate iteration purely on the bounds check
'odata + olen <= end'. Callers see the same variables, in the same
order, just without the spurious type-0 truncation.
Assisted-by: Claude:claude-opus-4-7
Link: https://github.com/openwrt/odhcp6c/pull/160
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit 9177f236c2d74144a8a2cbdc7da0fcf2f582bc2c)
---
src/odhcp6c.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
--- a/src/odhcp6c.h
+++ b/src/odhcp6c.h
@@ -267,8 +267,10 @@ struct dhcpv6_s46_rule {
#define dhcpv6_for_each_option(start, end, otype, olen, odata)\
for (uint8_t *_o = (uint8_t*)(start); _o + 4 <= (uint8_t*)(end) &&\
- ((otype) = _o[0] << 8 | _o[1]) && ((odata) = (void*)&_o[4]) &&\
- ((olen) = _o[2] << 8 | _o[3]) + (odata) <= (uint8_t*)(end); \
+ ((otype) = _o[0] << 8 | _o[1],\
+ (odata) = (void*)&_o[4],\
+ (olen) = _o[2] << 8 | _o[3],\
+ (odata) + (olen) <= (uint8_t*)(end)); \
_o += 4 + (_o[2] << 8 | _o[3]))
@@ -0,0 +1,91 @@
From 80c68194417ab73490fb99a8589d9c02d7fc5ede Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 11 Jul 2026 23:38:11 +0200
Subject: dhcpv6: enforce monotonic replay counter on Reconfigure RKAP
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
RFC 8415 §20.4.3 requires the client to track the replay-detection
field of an authenticated Reconfigure message and to reject any
incoming Reconfigure whose replay value does not exceed the highest
one already accepted. Without this check, a captured Reconfigure can
be replayed at will: the HMAC is still valid because the message
itself has not changed, and the client will happily re-trigger
Renew / Rebind / Information-Request cycles on demand.
Add a per-binding (reconf_replay, reconf_replay_seen) tuple,
initialised in dhcpv6_promote_server_cand() alongside reconf_key so a
fresh binding starts with a fresh window. Compare the wire replay
field (network byte order, decoded with be64toh()) against the stored
value before running the HMAC verification; only commit the value on
a verified HMAC so a forged message cannot move the counter forward.
Assisted-by: Claude:claude-opus-4-7
Link: https://github.com/openwrt/odhcp6c/pull/160
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit df27a49c98d89cb945767763a47bfafe196165c8)
[24.10 has no per-candidate reconf_key and no auth_protocol selector: the replay
state is global alongside the existing global reconf_key, and is reset in
dhcpv6_promote_server_cand() when a new server binding is established]
---
src/dhcpv6.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
--- a/src/dhcpv6.c
+++ b/src/dhcpv6.c
@@ -15,6 +15,7 @@
#include <time.h>
#include <fcntl.h>
+#include <endian.h>
#include <errno.h>
#include <inttypes.h>
#include <stdlib.h>
@@ -111,6 +112,8 @@ static struct in6_addr server_addr = IN6
// Reconfigure key
static uint8_t reconf_key[16];
+static uint64_t reconf_replay;
+static bool reconf_replay_seen;
// client options
static unsigned int client_options = 0;
@@ -827,6 +830,16 @@ static bool dhcpv6_response_is_valid(con
if (r->protocol != 3 || r->algorithm != 1 || r->reconf_type != 2)
continue;
+ /* RFC 8415 §20.4.3: the replay-detection field must be
+ * monotonically increasing per (client, server-id) pair.
+ * Drop any Reconfigure whose replay value does not exceed
+ * the highest one we have already accepted. */
+ uint64_t replay;
+ memcpy(&replay, &r->replay, sizeof(replay));
+ replay = be64toh(replay);
+ if (reconf_replay_seen && replay <= reconf_replay)
+ continue;
+
md5_ctx_t md5;
uint8_t serverhash[16], secretbytes[64];
uint32_t hash[4];
@@ -855,6 +868,10 @@ static bool dhcpv6_response_is_valid(con
md5_end(hash, &md5);
rcauth_ok = !memcmp(hash, serverhash, sizeof(hash));
+ if (rcauth_ok) {
+ reconf_replay = replay;
+ reconf_replay_seen = true;
+ }
} else if (otype == DHCPV6_OPT_RECONF_MESSAGE && olen == 1) {
rcmsg = odata[0];
} else if ((otype == DHCPV6_OPT_IA_PD || otype == DHCPV6_OPT_IA_NA)) {
@@ -1680,6 +1697,8 @@ int dhcpv6_promote_server_cand(void)
odhcp6c_add_state(STATE_SERVER_ID, hdr, sizeof(hdr));
odhcp6c_add_state(STATE_SERVER_ID, cand->duid, cand->duid_len);
accept_reconfig = cand->wants_reconfigure;
+ reconf_replay = 0;
+ reconf_replay_seen = false;
if (cand->ia_na_len) {
odhcp6c_add_state(STATE_IA_NA, cand->ia_na, cand->ia_na_len);
@@ -0,0 +1,120 @@
From 35f1c268c8ffb2bfe2c7f4fa0415215ea532570b Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 11 Jul 2026 23:47:42 +0200
Subject: odhcp6c: fix handling of RFC6603 Prefix Exclude Option
Several bugs in the encoding and (more importantly) decoding of
DHCPV6_OPT_PD_EXCLUDE lead to the option being ignored in some received
messages, or the excluded subnet id being mangled to an incorrect value.
Fix both encoding and decoding, being more explicit and slightly more
verbose for clarity.
Signed-off-by: Mark H. Spatz <mark.h.spatz@gmail.com>
Link: https://github.com/openwrt/odhcp6c/pull/151
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit 07d324ee7222c0e15b9975281f18236fdccc11bd)
[24.10 has no dedicated exclusion_length field (dcb53c4 is not backported), so the
port keeps reusing entry.priority / e[].priority for the exclusion length. The
upstream 'slen > 2' -> 'slen >= 2' guard change is a no-op here: 24.10 already
skips on 'slen < 2'. The added info() PD_EXCLUDE trace is dropped, as the log
helpers from bfd7597 are not backported. The computed option length is unchanged
(excl_opt_len == old ex_len), so the ia_pd buffer sizing is unaffected.]
---
src/dhcpv6.c | 47 +++++++++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 20 deletions(-)
--- a/src/dhcpv6.c
+++ b/src/dhcpv6.c
@@ -437,13 +437,16 @@ static void dhcpv6_send(enum dhcpv6_msg
if (e[j].iaid != iaid)
continue;
- uint8_t ex_len = 0;
- if (e[j].priority > 0)
- ex_len = ((e[j].priority - e[j].length - 1) / 8) + 6;
+ uint8_t excl_subnet_id_nbits, excl_subnet_id_nbytes, excl_opt_len = 0;
+ if (e[j].priority > 0) {
+ excl_subnet_id_nbits = e[j].priority - e[j].length;
+ excl_subnet_id_nbytes = ((excl_subnet_id_nbits - 1) / 8) + 1;
+ excl_opt_len = excl_subnet_id_nbytes + 4 + 1;
+ }
struct dhcpv6_ia_prefix p = {
.type = htons(DHCPV6_OPT_IA_PREFIX),
- .len = htons(sizeof(p) - 4U + ex_len),
+ .len = htons(sizeof(p) - 4U + excl_opt_len),
.prefix = e[j].length,
.addr = e[j].target
};
@@ -456,20 +459,22 @@ static void dhcpv6_send(enum dhcpv6_msg
memcpy(ia_pd + ia_pd_len, &p, sizeof(p));
ia_pd_len += sizeof(p);
- if (ex_len) {
+ if (excl_opt_len) {
ia_pd[ia_pd_len++] = 0;
ia_pd[ia_pd_len++] = DHCPV6_OPT_PD_EXCLUDE;
ia_pd[ia_pd_len++] = 0;
- ia_pd[ia_pd_len++] = ex_len - 4;
+ ia_pd[ia_pd_len++] = excl_opt_len - 4;
ia_pd[ia_pd_len++] = e[j].priority;
- uint32_t excl = ntohl(e[j].router.s6_addr32[1]);
- excl >>= (64 - e[j].priority);
- excl <<= 8 - ((e[j].priority - e[j].length) % 8);
-
- for (size_t i = ex_len - 5; i > 0; --i, excl >>= 8)
- ia_pd[ia_pd_len + i] = excl & 0xff;
- ia_pd_len += ex_len - 5;
+ uint32_t excluded_bits = ntohl(e[j].router.s6_addr32[1]);
+ excluded_bits >>= (64 - e[j].priority); /* Right align subnet ID bits */
+ excluded_bits <<= (32 - excl_subnet_id_nbits); /* Left align subnet ID bits */
+
+ /* Copy subnet ID bits into the option MSB first */
+ for (size_t k = 0; k < excl_subnet_id_nbytes; ++k) {
+ ia_pd[ia_pd_len++] = excluded_bits >> 24;
+ excluded_bits <<= 8;
+ }
}
hdr->len = htons(ntohs(hdr->len) + ntohs(p.len) + 4U);
@@ -1408,7 +1413,9 @@ static unsigned int dhcpv6_parse_ia(void
if (stype != DHCPV6_OPT_PD_EXCLUDE || slen < 2)
continue;
+ /* RFC 6603 §4.2 Prefix Exclude option */
uint8_t elen = sdata[0];
+ uint8_t *excl_subnet_id = &sdata[1];
if (elen > 64)
elen = 64;
@@ -1417,19 +1424,19 @@ static unsigned int dhcpv6_parse_ia(void
continue;
}
- uint8_t bytes = ((elen - entry.length - 1) / 8) + 1;
- if (slen <= bytes) {
+ uint8_t excl_subnet_id_nbits = elen - entry.length;
+ uint8_t excl_subnet_id_nbytes = ((excl_subnet_id_nbits - 1) / 8) + 1;
+ if ((excl_subnet_id + excl_subnet_id_nbytes) > (sdata + slen)) {
ok = false;
continue;
}
uint32_t exclude = 0;
- do {
- exclude = exclude << 8 | sdata[bytes];
- } while (--bytes);
-
- exclude >>= 8 - ((elen - entry.length) % 8);
- exclude <<= 64 - elen;
+ /* Copy subnet ID bits out of the option MSB first */
+ for (size_t k = 0; k < excl_subnet_id_nbytes; k++)
+ exclude = (exclude << 8) | excl_subnet_id[k];
+ exclude >>= (8 * excl_subnet_id_nbytes) - excl_subnet_id_nbits; /* Right align subnet ID bits */
+ exclude <<= (64 - elen); /* Shift subnet ID bits into the low-order bits of the prefix */
// Abusing router & priority fields for exclusion
entry.router = entry.target;