uhttpd: cherry pick patches from main

This fixes the following security isseus with CVE numbers:
CVE-2026-55614: GHSA-mcfg-c4r7-pjpf — case-sensitive Transfer-Encoding matching
CVE-2026-55612: GHSA-p55c-rmhc-qfm5 — invalid chunk-length state reset
CVE-2026-55613: GHSA-wgwp-64hh-f52p — ubus POST body parse-error desync

The branch also carries hardening with no CVE: one-byte overflow in
uh_urldecode, off-by-one OOB read in uh_b64decode, constant-time
password compare, $p$ crypt-hash handling.

Link: https://github.com/openwrt/openwrt/pull/24259
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit f68fa732873c280f290f16aef32b3240c790b390)
This commit is contained in:
Hauke Mehrtens 2026-07-16 22:48:21 +02:00 committed by ahmed sassi
parent 328e3c3f35
commit 9287454eee
18 changed files with 894 additions and 1 deletions

View File

@ -8,7 +8,7 @@
include $(TOPDIR)/rules.mk
PKG_NAME:=uhttpd
PKG_RELEASE:=4
PKG_RELEASE:=5
PKG_SOURCE_PROTO:=git
PKG_SOURCE_URL=$(PROJECT_GIT)/project/uhttpd.git

View File

@ -0,0 +1,44 @@
From ea8b2951efa484200bb8027fd44b67a672c89f49 Mon Sep 17 00:00:00 2001
From: Tito Brasolin <tito.brasolin@kerberos.energy>
Date: Fri, 5 Sep 2025 10:09:16 +0200
Subject: ubus: unregister ubus subscriber on HTTP client disconnect
Fixes a potential SIGSEGV when a client disconnects from a /ubus/subscribe/... endpoint without unsubscribing.
The ubus subscriber is now properly unregistered in a cleanup handler, preventing callbacks on freed client structures.
Fixes: #1
Signed-off-by: Tito Brasolin <tito.brasolin@kerberos.energy>
Link: https://github.com/openwrt/uhttpd/pull/18
Signed-off-by: Robert Marko <robert.marko@sartura.hr>
(cherry picked from commit 506e24987b97fbc866005bfb71316bd63601a1ef)
---
ubus.c | 11 +++++++++++
1 file changed, 11 insertions(+)
--- a/ubus.c
+++ b/ubus.c
@@ -362,6 +362,14 @@ static void uh_ubus_subscription_notific
ops->request_done(cl);
}
+/* Cleanup function to unregister ubus subscriber when HTTP client closes */
+static void uh_ubus_subscription_free(struct client *cl)
+{
+ struct dispatch_ubus *du = &cl->dispatch.ubus;
+ if (du->sub.obj.id)
+ ubus_unregister_subscriber(ctx, &du->sub);
+}
+
static void uh_ubus_handle_get_subscribe(struct client *cl, const char *path)
{
struct dispatch_ubus *du = &cl->dispatch.ubus;
@@ -399,6 +407,9 @@ static void uh_ubus_handle_get_subscribe
if (conf.events_retry)
ops->chunk_printf(cl, "retry: %d\n", conf.events_retry);
+ /* Ensure cleanup on client disconnect */
+ cl->dispatch.free = uh_ubus_subscription_free;
+
return;
err_unregister:

View File

@ -0,0 +1,32 @@
From db284b69ef64be20d45a12c3de024ab91fe3dc61 Mon Sep 17 00:00:00 2001
From: Paul Spooren <mail@aparcar.org>
Date: Thu, 16 Apr 2026 17:10:36 +0800
Subject: client: use base-10 parsing for Content-Length header
strtoul() with base 0 auto-detects octal (leading "0") and
hexadecimal (leading "0x") prefixes. A Content-Length value like
"025" was parsed as octal 21 instead of decimal 25. Since compliant
HTTP frontends always parse Content-Length as decimal per RFC 7230,
this mismatch enables HTTP request smuggling when uhttpd sits behind
a reverse proxy or load balancer.
Fix by explicitly passing base 10 to strtoul().
Reported-by: Nicola Staller <nicola.staller@syss.de>
Signed-off-by: Paul Spooren <mail@aparcar.org>
(cherry picked from commit e619cb04cddba8316d6928ff99f55a49e6ddc561)
---
client.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/client.c
+++ b/client.c
@@ -391,7 +391,7 @@ static void client_parse_header(struct c
return;
}
} else if (!strcmp(data, "content-length")) {
- r->content_length = strtoul(val, &err, 0);
+ r->content_length = strtoul(val, &err, 10);
if ((err && *err) || r->content_length < 0) {
uh_header_error(cl, 400, "Bad Request");
return;

View File

@ -0,0 +1,40 @@
From e4b37d422219895d0f0bba75e49c9ccff0bc5936 Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sun, 3 May 2026 22:47:29 +0200
Subject: client: prevent transfer_chunked counter overflow
The transfer_chunked field is a uint8_t used as a tri-state marker:
0 means no chunked transfer, 1 means the first chunk header is next
(no leading CRLF), and any value greater than 1 means a subsequent
chunk header is next (expect a leading CRLF).
The counter was incremented unconditionally on every parsed chunk,
so after 255 chunks it wrapped back to 0. The outer loop then treats
a wrapped value as "not chunked" and signals end-of-body, while
later bytes on the wire are interpreted as the next pipelined
request. An attacker could exploit this for HTTP request smuggling
on a keep-alive connection by splitting a body into 256 or more
small chunks.
Cap the counter at 2 once the "subsequent chunk" state has been
reached, since further increments are not meaningful.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit d2551871b5e52b576236c0a2470f392eca81c2de)
---
client.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/client.c
+++ b/client.c
@@ -487,7 +487,8 @@ void client_poll_post_data(struct client
*sep = 0;
r->content_length = strtoul(buf + offset, &sep, 16);
- r->transfer_chunked++;
+ if (r->transfer_chunked < 2)
+ r->transfer_chunked++;
ustream_consume(cl->us, sep + 2 - buf);
/* invalid chunk length */

View File

@ -0,0 +1,44 @@
From ca7e659b807864b713ea449cd46110ac28a8e389 Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sun, 3 May 2026 22:49:47 +0200
Subject: client: match Host and URL attributes exactly in tls_redirect_check
The TLS redirect logic walked the header blob with strncmp(name,
\"host\", 4) and strncmp(name, \"URL\", 3), so any header whose
lowercased name started with \"host\" (for example \"host-foo\" or
\"hostess\") would be picked up as the request authority, and any
attribute starting with \"URL\" would be picked up as the request
target.
The Host value is then echoed verbatim into the Location header of
the 307 response. A client without a real Host header but with
\"Hostess: attacker.example\" would therefore receive a redirect
pointing at the attacker, turning the TLS-redirect feature into an
open redirect.
Use strcmp so only the canonical \"host\" header (lowercased by
client_parse_header) and the synthetic \"URL\" attribute set by
client_parse_request can match.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit 07f0afb3bf9160b90111009829676da1758060ff)
---
client.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/client.c
+++ b/client.c
@@ -294,10 +294,10 @@ static bool tls_redirect_check(struct cl
return true;
blob_for_each_attr(cur, cl->hdr.head, rem) {
- if (!strncmp(blobmsg_name(cur), "host", 4))
+ if (!strcmp(blobmsg_name(cur), "host"))
host = blobmsg_get_string(cur);
- if (!strncmp(blobmsg_name(cur), "URL", 3))
+ if (!strcmp(blobmsg_name(cur), "URL"))
url = blobmsg_get_string(cur);
if (url && host)

View File

@ -0,0 +1,74 @@
From 5ddd2589ac62bb8d02f5c3f53704adb0f09a2f2f Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sun, 3 May 2026 23:53:37 +0200
Subject: proc: store CGI Status message per-client instead of in a shared
buffer
proc_handle_header parsed the CGI \"Status:\" header into a function-
static char status_buf[64] and pointed cl->dispatch.proc.status_msg at
it. dispatch_proc.status_msg was a plain char *, so every client's
status_msg ended up aliasing the same static buffer.
Header parsing for one CGI is split across multiple relay_read_cb
invocations whenever the child sends headers in chunks. Between two
reads of the same client, uloop can dispatch another client's CGI,
whose proc_handle_header overwrites the shared status_buf. When the
first client's empty-line read finally arrives and proc_handle_header_end
fires, status_msg points at the second client's text, so the
\"HTTP/1.1 <code> <msg>\" status line emitted to the first client
mixes its own status_code with another client's reason phrase.
Replace the char * with a fixed-size char status_msg[64] embedded in
dispatch_proc and snprintf into it directly. Each client now owns its
own buffer, so concurrent CGI responses can no longer trample each
other's status line.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit 05317bf30a9432aaeac1a20300f605d9cf43c64f)
---
proc.c | 7 +++----
uhttpd.h | 2 +-
2 files changed, 4 insertions(+), 5 deletions(-)
--- a/proc.c
+++ b/proc.c
@@ -201,7 +201,6 @@ static void proc_handle_close(struct rel
static void proc_handle_header(struct relay *r, const char *name, const char *val)
{
- static char status_buf[64];
struct client *cl = r->cl;
char *sep;
char buf[4];
@@ -213,8 +212,8 @@ static void proc_handle_header(struct re
memcpy(buf, val, 3);
buf[3] = 0;
- snprintf(status_buf, sizeof(status_buf), "%s", sep + 1);
- cl->dispatch.proc.status_msg = status_buf;
+ snprintf(cl->dispatch.proc.status_msg,
+ sizeof(cl->dispatch.proc.status_msg), "%s", sep + 1);
cl->dispatch.proc.status_code = atoi(buf);
return;
}
@@ -337,7 +336,7 @@ bool uh_create_process(struct client *cl
blob_buf_init(&proc->hdr, 0);
proc->status_code = 200;
- proc->status_msg = "OK";
+ strcpy(proc->status_msg, "OK");
if (pipe(rfd))
return false;
--- a/uhttpd.h
+++ b/uhttpd.h
@@ -206,7 +206,7 @@ struct dispatch_proc {
struct uloop_fd wrfd;
struct relay r;
int status_code;
- char *status_msg;
+ char status_msg[64];
};
struct dispatch_handler {

View File

@ -0,0 +1,74 @@
From a0dfee524f5bf3fa95851f30cfb4edbf23b0bcdf Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sun, 3 May 2026 23:56:57 +0200
Subject: utils, client: cast char to unsigned before passing to ctype
functions
The is*() and tolower()/toupper() functions accept an int argument
that must be either EOF or representable as unsigned char. Passing a
plain char with the high bit set produces a sign-extended negative
int on platforms where char is signed (the default on x86), which is
undefined behavior per C99 / POSIX. Real-world libc implementations
typically resolve the negative index against an internal lookup table
and may return surprising results or read out of bounds.
Inputs reach these functions from URL bytes, header values, and the
gap between a header name and its value, all of which can legitimately
contain bytes >= 0x80 (UTF-8 paths, latin-1 percent-decoded values,
etc.).
Cast through unsigned char at every call site so the value is in the
range [0, UCHAR_MAX] regardless of the platform's char signedness.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit 1781b6dec414a4e284aacf09a544d58fac374289)
---
client.c | 4 ++--
utils.c | 8 +++++---
2 files changed, 7 insertions(+), 5 deletions(-)
--- a/client.c
+++ b/client.c
@@ -380,8 +380,8 @@ static void client_parse_header(struct c
}
for (name = data; *name; name++)
- if (isupper(*name))
- *name = tolower(*name);
+ if (isupper((unsigned char)*name))
+ *name = tolower((unsigned char)*name);
if (!strcmp(data, "expect")) {
if (!strcasecmp(val, "100-continue"))
--- a/utils.c
+++ b/utils.c
@@ -119,7 +119,9 @@ int uh_urldecode(char *buf, int blen, co
continue;
}
- if (i + 2 >= slen || !isxdigit(src[i + 1]) || !isxdigit(src[i + 2]))
+ if (i + 2 >= slen ||
+ !isxdigit((unsigned char)src[i + 1]) ||
+ !isxdigit((unsigned char)src[i + 2]))
return -2;
buf[len++] = (char)(16 * hex(src[i+1]) + hex(src[i+2]));
@@ -141,7 +143,7 @@ int uh_urlencode(char *buf, int blen, co
for (i = 0; (i < slen) && (len < blen); i++)
{
- if( isalnum(src[i]) || (src[i] == '-') || (src[i] == '_') ||
+ if( isalnum((unsigned char)src[i]) || (src[i] == '-') || (src[i] == '_') ||
(src[i] == '.') || (src[i] == '~') )
{
buf[len++] = src[i];
@@ -231,7 +233,7 @@ char *uh_split_header(char *str)
*val = 0;
val++;
- while (isspace(*val))
+ while (isspace((unsigned char)*val))
val++;
return val;

View File

@ -0,0 +1,68 @@
From 86d303781a68278512e116cc0fc02ec7c43f1f1b Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Mon, 4 May 2026 00:22:00 +0200
Subject: utils: fix one-byte overflow in uh_urldecode
The output buffer was filled up to blen bytes without writing a
trailing NUL, leaving every caller to either append one or rely on
a happenstance zero byte. The documented contract said "not
null-terminated" but every in-tree caller used the result as a C
string, making the missing NUL a latent bug.
Reserve one byte at the tail, write the NUL, and update the contract
in the comment. Reject blen <= 0 as overflow.
Adjust the -d option's caller to pass the actual buffer size
(strlen + 1) rather than strlen -- the buffer was always sized for
the NUL, only the blen argument was off-by-one. Without this the -d
handler would reject every plain-ASCII input as "invalid encoding"
after the contract change.
Also cast ctype arguments to unsigned char to avoid undefined
behavior on signed-char platforms.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit ced7b15c346741982fe317cd2d9de0895e6c64a9)
---
main.c | 2 +-
utils.c | 10 +++++++---
2 files changed, 8 insertions(+), 4 deletions(-)
--- a/main.c
+++ b/main.c
@@ -453,7 +453,7 @@ int main(int argc, char **argv)
optarg[opt] = ' ';
/* opt now contains strlen(optarg) -- no need to re-scan */
- if (uh_urldecode(port, opt, optarg, opt) < 0) {
+ if (uh_urldecode(port, opt + 1, optarg, opt) < 0) {
fprintf(stderr, "uhttpd: invalid encoding\n");
return -1;
}
--- a/utils.c
+++ b/utils.c
@@ -100,8 +100,9 @@ void uh_chunk_eof(struct client *cl)
}
/* blen is the size of buf; slen is the length of src. The input-string need
-** not be, and the output string will not be, null-terminated. Returns the
-** length of the decoded string, -1 on buffer overflow, -2 on malformed string. */
+** not be null-terminated. The output string is always null-terminated when
+** blen > 0. Returns the length of the decoded string (excluding the trailing
+** NUL), -1 on buffer overflow, -2 on malformed string. */
int uh_urldecode(char *buf, int blen, const char *src, int slen)
{
int i;
@@ -112,7 +113,10 @@ int uh_urldecode(char *buf, int blen, co
(((x) <= 'F') ? ((x) - 'A' + 10) : \
((x) - 'a' + 10)))
- for (i = 0; (i < slen) && (len < blen); i++)
+ if (blen <= 0)
+ return -1;
+
+ for (i = 0; (i < slen) && (len < blen - 1); i++)
{
if (src[i] != '%') {
buf[len++] = src[i];

View File

@ -0,0 +1,35 @@
From d97b4ed70fa15f5853386ef89eefe37a5ca83935 Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Mon, 4 May 2026 00:23:38 +0200
Subject: file: bail out of file_write_cb on read error
The static-file delivery loop in file_write_cb only handled the
EINTR branch of a failed read(); other failure modes (EIO on a
backing storage error, EBADF on an unexpectedly-closed fd, etc.)
fell through with r < 0 and reached uh_chunk_write(cl, uh_buf, r),
which forwards the negative length to ustream_write() where it is
interpreted as an unsigned size. The result is a chunked transfer
of arbitrary memory adjacent to uh_buf to the client.
On any non-EINTR read error, terminate the request like we already
do for EOF instead of writing past the buffer.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit 53e7150619a3662226d1de408c5ac51f859c47f2)
---
file.c | 3 +++
1 file changed, 3 insertions(+)
--- a/file.c
+++ b/file.c
@@ -581,6 +581,9 @@ static void file_write_cb(struct client
if (r < 0) {
if (errno == EINTR)
continue;
+
+ uh_request_done(cl);
+ return;
}
if (!r) {

View File

@ -0,0 +1,36 @@
From eb45f0c0534656f63b001eef32d28a9b71705433 Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Mon, 13 Apr 2026 10:25:23 +0200
Subject: utils: fix off-by-one out-of-bounds read in uh_b64decode
The loop condition used `i <= slen`, which causes `str[slen]` to be
read after processing all `slen` bytes. This is one byte past the
end of the declared input length and constitutes an out-of-bounds
read for any caller that passes a non-null-terminated buffer.
The existing call site in auth.c happens to pass a null-terminated C
string, so `str[slen]` equals '\0' and the loop terminates naturally
without memory corruption in practice. However the function
signature advertises only `slen` bytes of valid input, so the read is
still incorrect.
Fix by changing the condition to `i < slen`.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit add5389470f0f5534a1b7ef79f69e98c95a5ba82)
---
utils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/utils.c
+++ b/utils.c
@@ -176,7 +176,7 @@ int uh_b64decode(char *buf, int blen, co
int len = 0;
int i = 0;
- for (i = 0; (i <= slen) && (str[i] != 0); i++)
+ for (i = 0; (i < slen) && (str[i] != 0); i++)
{
cin = str[i];

View File

@ -0,0 +1,53 @@
From b81ef5a9b430dbe3829d16a04bc10222ab69a53a Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Tue, 31 Mar 2026 22:56:15 +0200
Subject: client: parse Content-Length safely
content_length is declared int but the previous strtoul cast silently
wrapped negative inputs into huge positive values, making the < 0
guard unreachable. Switching to strtol exposes negatives, but on
64-bit platforms a value above INT_MAX still passes strtol's own
range check and truncates implementation-defined when narrowed to int.
Parse into a long, check errno for strtol overflow, and reject any
value above INT_MAX or below zero before assigning. The error path
returns 400 Bad Request as before.
Pull in errno.h and limits.h for the new checks.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit 2c869c094c25e1b6b7c1ba786f22280a6c153447)
---
client.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
--- a/client.c
+++ b/client.c
@@ -19,6 +19,8 @@
#include <libubox/blobmsg.h>
#include <ctype.h>
+#include <errno.h>
+#include <limits.h>
#include "uhttpd.h"
#include "tls.h"
@@ -391,11 +393,15 @@ static void client_parse_header(struct c
return;
}
} else if (!strcmp(data, "content-length")) {
- r->content_length = strtoul(val, &err, 10);
- if ((err && *err) || r->content_length < 0) {
+ long length;
+
+ errno = 0;
+ length = strtol(val, &err, 10);
+ if ((err && *err) || errno || length < 0 || length > INT_MAX) {
uh_header_error(cl, 400, "Bad Request");
return;
}
+ r->content_length = (int)length;
} else if (!strcmp(data, "transfer-encoding")) {
if (!strcmp(val, "chunked"))
r->transfer_chunked = true;

View File

@ -0,0 +1,60 @@
From 478e959e05e1a5fdecb49fee2ae54f265bb074c2 Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Tue, 31 Mar 2026 22:56:47 +0200
Subject: client: parse chunked transfer chunk size safely
The chunk-size hex field is parsed into content_length (int). The
previous strtoul could wrap negative-looking inputs into huge
positive values; switching to strtol exposes negatives but leaves
the 64-bit INT_MAX overflow window open, where a long value above
INT_MAX truncates implementation-defined when narrowed.
Parse into a long, check errno for strtol overflow, and reject any
chunk size above INT_MAX. On malformed or out-of-range input the
chunked transfer state is torn down as before.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit 9404e6c62bb7dc452a294a62c33eb18beb9ef433)
---
client.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
--- a/client.c
+++ b/client.c
@@ -457,6 +457,7 @@ void client_poll_post_data(struct client
while (1) {
char *sep;
+ long chunk_len;
int offset = 0;
int cur_len;
@@ -492,18 +493,22 @@ void client_poll_post_data(struct client
*sep = 0;
- r->content_length = strtoul(buf + offset, &sep, 16);
- if (r->transfer_chunked < 2)
- r->transfer_chunked++;
- ustream_consume(cl->us, sep + 2 - buf);
+ errno = 0;
+ chunk_len = strtol(buf + offset, &sep, 16);
/* invalid chunk length */
- if ((sep && *sep) || r->content_length < 0) {
+ if ((sep && *sep) || errno || chunk_len < 0 || chunk_len > INT_MAX) {
+ ustream_consume(cl->us, sep + 2 - buf);
r->content_length = 0;
r->transfer_chunked = 0;
break;
}
+ if (r->transfer_chunked < 2)
+ r->transfer_chunked++;
+ ustream_consume(cl->us, sep + 2 - buf);
+ r->content_length = (int)chunk_len;
+
/* empty chunk == eof */
if (!r->content_length) {
r->transfer_chunked = false;

View File

@ -0,0 +1,46 @@
From 62708e5be152027df9f0a844f6a6090a56c166d2 Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sun, 3 May 2026 22:54:03 +0200
Subject: auth: do not accept stored crypt hash as plaintext password
uh_auth_check first compared the supplied password to the stored
credential with strcmp() and only fell through to crypt() on
mismatch. The strcmp branch is meant to support plaintext
credentials in /etc/httpd.conf, but it also matched when the stored
credential is a crypt hash and the request supplies that exact hash
as the password.
When the password line uses the '$p$<user>' indirection, realm->pass
is populated from /etc/shadow or /etc/passwd. Anyone who manages to
read the hash (a backup, an unrelated disclosure bug, an offline
copy) can therefore authenticate by sending the hash itself,
bypassing the need to recover the underlying password.
Skip the plaintext compare when realm->pass starts with '$', which
is the marker for every modular crypt(3) format (md5crypt, sha256,
sha512, bcrypt, yescrypt, ...). Plaintext credentials configured
via httpd.conf still work; stored hashes are only honored through
crypt().
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit b33ca5d377189f0fa7f6ee1b325ec08078a64c91)
---
auth.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
--- a/auth.c
+++ b/auth.c
@@ -125,8 +125,11 @@ bool uh_auth_check(struct client *cl, co
if (!req->realm)
return true;
+ /* The leading '$' check distinguishes a stored plaintext password from a
+ * modern crypt(3) hash ($id$salt$...). It blocks the case where a client
+ * sends the stored hash itself as the password and matches via strcmp. */
if (user_match &&
- (!strcmp(pass, realm->pass) ||
+ ((realm->pass[0] != '$' && !strcmp(pass, realm->pass)) ||
!strcmp(crypt(pass, realm->pass), realm->pass))) {
if (uptr)
*uptr = user;

View File

@ -0,0 +1,62 @@
From e5422889da37ee164ca4913d5c2968bce6a18a58 Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Tue, 31 Mar 2026 22:58:00 +0200
Subject: auth: replace strcmp with constant-time password comparison
strcmp short-circuits on the first differing byte, leaking password
match progress via measurable response time differences. Add
uh_pass_compare() which XORs all bytes unconditionally and only
returns true when both length and content match, preventing a
timing-based password oracle attack.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit 6fadf0da50509a510ac4f85d2adb70a83de2e1fc)
---
auth.c | 28 ++++++++++++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
--- a/auth.c
+++ b/auth.c
@@ -25,6 +25,30 @@
#endif
#include "uhttpd.h"
+/*
+ * Constant-time string comparison to prevent timing-based password oracle.
+ * Returns true only if both strings are equal in length and content.
+ */
+static bool uh_pass_compare(const char *a, const char *b)
+{
+ unsigned int diff = 0;
+ size_t la, lb, i, maxlen;
+
+ if (!a || !b)
+ return false;
+
+ la = strlen(a);
+ lb = strlen(b);
+ maxlen = la > lb ? la : lb;
+
+ for (i = 0; i < maxlen; i++)
+ diff |= (unsigned char)(i < la ? a[i] : 0) ^
+ (unsigned char)(i < lb ? b[i] : 0);
+
+ diff |= (unsigned int)(la ^ lb);
+ return diff == 0;
+}
+
static LIST_HEAD(auth_realms);
void uh_auth_add(const char *path, const char *user, const char *pass)
@@ -129,8 +153,8 @@ bool uh_auth_check(struct client *cl, co
* modern crypt(3) hash ($id$salt$...). It blocks the case where a client
* sends the stored hash itself as the password and matches via strcmp. */
if (user_match &&
- ((realm->pass[0] != '$' && !strcmp(pass, realm->pass)) ||
- !strcmp(crypt(pass, realm->pass), realm->pass))) {
+ ((realm->pass[0] != '$' && uh_pass_compare(pass, realm->pass)) ||
+ uh_pass_compare(crypt(pass, realm->pass), realm->pass))) {
if (uptr)
*uptr = user;

View File

@ -0,0 +1,98 @@
From ce58f84074b9fe905be9c60e704312518a891257 Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Mon, 18 May 2026 00:01:16 +0200
Subject: auth: classify $p$ lookups by account state
uh_auth_add resolves '$p$<user>' password lines by reading the named
account's stored credential from /etc/shadow (or /etc/passwd as a
fallback). The previous code copied that string verbatim into
realm->pass without checking whether it was a usable crypt(3) hash.
This broke for two distinct shadow states:
1. Locked or placeholder credentials ("*", "x", "!", "!!", "*LK*",
"*NP*", "!hash" lock prefix). In uh_auth_check the plaintext
compare branch fires whenever realm->pass does not start with '$',
so any client sending the placeholder verbatim as the password
matched and authenticated. On OpenWrt every system account except
root has '*' or 'x' in /etc/shadow by default, so a realm configured as
'/cgi-bin/admin:adminuser:$p$daemon' (the pattern the example UCI
in package/network/services/uhttpd/files/uhttpd.config documents)
authenticated any request with password '*' and ran the CGI as
root.
2. Empty password (the default OpenWrt root entry, "root:::..."):
handled correctly by the existing empty-pass drop but silently,
leaving admins unaware that '$p$root' on a fresh image produces a
public URL.
Match login(1) semantics: locked or placeholder accounts deny all
access, accounts with no password set permit access without
authentication, accounts with a real hash require it. For each
non-hash case log a distinct warning so admins notice the silent
state. A '$p$' reference to a non-existent account is treated as a
config typo: deny all access (consistent with the admin's stated
intent of requiring auth) and log a warning.
For the locked case, bind the realm to a sentinel that starts with
'$' (skipping the plaintext compare branch) and cannot be produced by
crypt(3) (failing the hash branch), so every request returns 401
instead of falling through to a missing-realm public response.
Reported-by: Amit Pinchasi <amitpinchasi123@gmail.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Link: https://github.com/openwrt/uhttpd/pull/25
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit 1b624f8f814ed568608d756512892416e0431d77)
---
auth.c | 38 ++++++++++++++++++++++++++++++++++++--
1 file changed, 36 insertions(+), 2 deletions(-)
--- a/auth.c
+++ b/auth.c
@@ -72,10 +72,44 @@ void uh_auth_add(const char *path, const
#endif
if (!new_pass) {
pwd = getpwnam(&pass[3]);
- if (pwd && pwd->pw_passwd && pwd->pw_passwd[0] &&
- pwd->pw_passwd[0] != '!')
+ if (pwd && pwd->pw_passwd && pwd->pw_passwd[0])
new_pass = pwd->pw_passwd;
}
+
+ /* Only honour a real crypt(3) hash. Anything else from shadow
+ * splits into two cases that mirror login(1) semantics:
+ *
+ * - Empty password field ("root:::..."): the system account
+ * permits passwordless login, so leave new_pass empty and
+ * let the realm be dropped at the next check, which leaves
+ * the protected path unauthenticated.
+ *
+ * - Any other non-'$' value ("*", "x", "!", "!!", "*LK*",
+ * "*NP*", "!hash" lock prefix, ...): the account is locked
+ * or its credential is a placeholder. Without this guard
+ * uh_auth_check's plaintext compare branch would fire (it
+ * runs whenever realm->pass does not start with '$') and a
+ * client could authenticate by sending the placeholder
+ * verbatim as the password. Bind the realm to a sentinel
+ * that starts with '$' (skipping the plaintext branch) and
+ * that crypt(3) cannot produce (failing the hash branch),
+ * so every request gets 401 instead of silent public
+ * access. */
+ if (!new_pass) {
+ fprintf(stderr, "uhttpd: account '%s' does not "
+ "exist; denying all access to %s\n",
+ &pass[3], path);
+ new_pass = "$";
+ } else if (new_pass[0] && new_pass[0] != '$') {
+ fprintf(stderr, "uhttpd: account '%s' is locked "
+ "or has a non-crypt password; denying all "
+ "access to %s\n", &pass[3], path);
+ new_pass = "$";
+ } else if (!new_pass[0]) {
+ fprintf(stderr, "uhttpd: account '%s' has no "
+ "password set; leaving %s unauthenticated\n",
+ &pass[3], path);
+ }
} else {
new_pass = pass;
}

View File

@ -0,0 +1,46 @@
From 6641b050e5f7c7fd9ae92e8e119d6d6dfc6e9837 Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 13 Jun 2026 02:40:15 +0200
Subject: client: reject unhandled Transfer-Encoding values
client_parse_header() lowercases header names but compared the
Transfer-Encoding value with a case-sensitive strcmp(val, "chunked").
RFC 9112 6.1 requires transfer-coding tokens to be matched
case-insensitively, so a request with "Transfer-Encoding: Chunked" was
accepted but chunked framing was never enabled. The declared body was
then left unread and, on a keep-alive connection, parsed as the next
HTTP request (request smuggling).
Match "chunked" with strcasecmp and reject any other transfer-coding we
do not implement with 501, instead of silently ignoring it and losing
track of the message framing.
Link: https://github.com/openwrt/uhttpd/security/advisories/GHSA-mcfg-c4r7-pjpf
Reported-by: @dyingc
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit ae015e099986ceace44975cb69629841a2d58d37)
---
client.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
--- a/client.c
+++ b/client.c
@@ -403,8 +403,16 @@ static void client_parse_header(struct c
}
r->content_length = (int)length;
} else if (!strcmp(data, "transfer-encoding")) {
- if (!strcmp(val, "chunked"))
+ /* RFC 9112 6.1: transfer-coding tokens are case-insensitive.
+ * Reject any coding we do not implement instead of silently
+ * ignoring it, otherwise the body framing would be unknown and
+ * the unread body could be parsed as the next request. */
+ if (!strcasecmp(val, "chunked")) {
r->transfer_chunked = true;
+ } else {
+ uh_header_error(cl, 501, "Not Implemented");
+ return;
+ }
} else if (!strcmp(data, "connection")) {
if (!strcasecmp(val, "close"))
r->connection_close = true;

View File

@ -0,0 +1,40 @@
From 34460b8211d676ef1b9f40d6b775c5e112933690 Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 13 Jun 2026 02:40:40 +0200
Subject: client: close connection on invalid chunk length
When client_poll_post_data() encounters an invalid chunk-length line it
consumes the chunk-size line and clears content_length and
transfer_chunked, which makes the request look complete. The remaining
buffered body was neither drained nor was connection_close set, so the
keep-alive parser re-entered CLIENT_STATE_INIT and parsed the leftover
body bytes as a new HTTP request (request smuggling). An oversized
chunk length (e.g. 80000000) can be used to align an embedded request
on the chunk boundary.
Force connection_close in the invalid chunk-length branch so the
out-of-sync connection is torn down instead of being reused.
Link: https://github.com/openwrt/uhttpd/security/advisories/GHSA-p55c-rmhc-qfm5
Reported-by: @dyingc
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit b78f518478794e16ba3568fc1258e40bf3e8eb3b)
---
client.c | 5 +++++
1 file changed, 5 insertions(+)
--- a/client.c
+++ b/client.c
@@ -509,6 +509,11 @@ void client_poll_post_data(struct client
ustream_consume(cl->us, sep + 2 - buf);
r->content_length = 0;
r->transfer_chunked = 0;
+ /* The remaining buffered body is left unconsumed and its
+ * framing is now unknown. Close the connection so the
+ * leftover bytes are not reparsed as the next request
+ * (request smuggling). */
+ r->connection_close = true;
break;
}

View File

@ -0,0 +1,41 @@
From b2285de129b9adc8b12f84f9a27c16c21f2b57a5 Mon Sep 17 00:00:00 2001
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 13 Jun 2026 02:40:48 +0200
Subject: ubus: close connection on POST body parse error
uh_ubus_data_send() is only entered while there is still POST body data
to read. When the JSON body is delivered in several TCP segments and a
complete object has already been parsed (du->jsobj set), or the body
exceeds UH_UBUS_MAX_POST_SIZE, the error path emits a JSON-RPC parse
error and returns 0 without consuming the remaining declared
Content-Length bytes. The caller neither drained the rest of the body
nor closed the connection, so the unread body suffix was parsed as a
new HTTP request on the same keep-alive connection (request smuggling).
Set connection_close in the error path so the out-of-sync connection is
torn down instead of being reused.
Link: https://github.com/openwrt/uhttpd/security/advisories/GHSA-wgwp-64hh-f52p
Reported-by: @dyingc
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit 7b1bec45826bd78c8afc993435bdc0f1df2fe399)
---
ubus.c | 6 ++++++
1 file changed, 6 insertions(+)
--- a/ubus.c
+++ b/ubus.c
@@ -942,6 +942,12 @@ static int uh_ubus_data_send(struct clie
return len;
error:
+ /* We abort parsing the request body here without consuming the
+ * remaining declared Content-Length bytes. Close the connection even
+ * when keep alive is set, as the unread body suffix would otherwise be
+ * interpreted as the start of the next request (request smuggling).
+ */
+ cl->request.connection_close = true;
uh_ubus_single_error(cl, ERROR_PARSE);
return 0;
}