diff options
| author | Hauke Mehrtens | 2026-07-16 20:48:21 +0000 |
|---|---|---|
| committer | Hauke Mehrtens | 2026-07-21 00:12:56 +0000 |
| commit | f68fa732873c280f290f16aef32b3240c790b390 (patch) | |
| tree | b8c86c25f0c5f8af5f56b62007dcd615f7951326 | |
| parent | 452bc24984b7df0ab12e73372d3fbb3930d404fe (diff) | |
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>
18 files changed, 894 insertions, 1 deletions
diff --git a/package/network/services/uhttpd/Makefile b/package/network/services/uhttpd/Makefile index 3899c58271..8db49a509a 100644 --- a/package/network/services/uhttpd/Makefile +++ b/package/network/services/uhttpd/Makefile @@ -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 diff --git a/package/network/services/uhttpd/patches/0001-ubus-unregister-ubus-subscriber-on-HTTP-client-disco.patch b/package/network/services/uhttpd/patches/0001-ubus-unregister-ubus-subscriber-on-HTTP-client-disco.patch new file mode 100644 index 0000000000..528d35b4e9 --- /dev/null +++ b/package/network/services/uhttpd/patches/0001-ubus-unregister-ubus-subscriber-on-HTTP-client-disco.patch @@ -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: diff --git a/package/network/services/uhttpd/patches/0002-client-use-base-10-parsing-for-Content-Length-header.patch b/package/network/services/uhttpd/patches/0002-client-use-base-10-parsing-for-Content-Length-header.patch new file mode 100644 index 0000000000..a83187a4aa --- /dev/null +++ b/package/network/services/uhttpd/patches/0002-client-use-base-10-parsing-for-Content-Length-header.patch @@ -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; diff --git a/package/network/services/uhttpd/patches/0003-client-prevent-transfer_chunked-counter-overflow.patch b/package/network/services/uhttpd/patches/0003-client-prevent-transfer_chunked-counter-overflow.patch new file mode 100644 index 0000000000..abd8e3455b --- /dev/null +++ b/package/network/services/uhttpd/patches/0003-client-prevent-transfer_chunked-counter-overflow.patch @@ -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 */ diff --git a/package/network/services/uhttpd/patches/0004-client-match-Host-and-URL-attributes-exactly-in-tls_.patch b/package/network/services/uhttpd/patches/0004-client-match-Host-and-URL-attributes-exactly-in-tls_.patch new file mode 100644 index 0000000000..9d520f8fa4 --- /dev/null +++ b/package/network/services/uhttpd/patches/0004-client-match-Host-and-URL-attributes-exactly-in-tls_.patch @@ -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) diff --git a/package/network/services/uhttpd/patches/0005-proc-store-CGI-Status-message-per-client-instead-of-.patch b/package/network/services/uhttpd/patches/0005-proc-store-CGI-Status-message-per-client-instead-of-.patch new file mode 100644 index 0000000000..774dc4c7e8 --- /dev/null +++ b/package/network/services/uhttpd/patches/0005-proc-store-CGI-Status-message-per-client-instead-of-.patch @@ -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 { diff --git a/package/network/services/uhttpd/patches/0006-utils-client-cast-char-to-unsigned-before-passing-to.patch b/package/network/services/uhttpd/patches/0006-utils-client-cast-char-to-unsigned-before-passing-to.patch new file mode 100644 index 0000000000..a8fa83e0fd --- /dev/null +++ b/package/network/services/uhttpd/patches/0006-utils-client-cast-char-to-unsigned-before-passing-to.patch @@ -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; diff --git a/package/network/services/uhttpd/patches/0007-utils-fix-one-byte-overflow-in-uh_urldecode.patch b/package/network/services/uhttpd/patches/0007-utils-fix-one-byte-overflow-in-uh_urldecode.patch new file mode 100644 index 0000000000..cf350b53ea --- /dev/null +++ b/package/network/services/uhttpd/patches/0007-utils-fix-one-byte-overflow-in-uh_urldecode.patch @@ -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]; diff --git a/package/network/services/uhttpd/patches/0008-file-bail-out-of-file_write_cb-on-read-error.patch b/package/network/services/uhttpd/patches/0008-file-bail-out-of-file_write_cb-on-read-error.patch new file mode 100644 index 0000000000..f8f462d174 --- /dev/null +++ b/package/network/services/uhttpd/patches/0008-file-bail-out-of-file_write_cb-on-read-error.patch @@ -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) { diff --git a/package/network/services/uhttpd/patches/0009-utils-fix-off-by-one-out-of-bounds-read-in-uh_b64dec.patch b/package/network/services/uhttpd/patches/0009-utils-fix-off-by-one-out-of-bounds-read-in-uh_b64dec.patch new file mode 100644 index 0000000000..ad17166afe --- /dev/null +++ b/package/network/services/uhttpd/patches/0009-utils-fix-off-by-one-out-of-bounds-read-in-uh_b64dec.patch @@ -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]; + diff --git a/package/network/services/uhttpd/patches/0010-client-parse-Content-Length-safely.patch b/package/network/services/uhttpd/patches/0010-client-parse-Content-Length-safely.patch new file mode 100644 index 0000000000..ffbbd659ba --- /dev/null +++ b/package/network/services/uhttpd/patches/0010-client-parse-Content-Length-safely.patch @@ -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; diff --git a/package/network/services/uhttpd/patches/0011-client-parse-chunked-transfer-chunk-size-safely.patch b/package/network/services/uhttpd/patches/0011-client-parse-chunked-transfer-chunk-size-safely.patch new file mode 100644 index 0000000000..d4d31d2b8f --- /dev/null +++ b/package/network/services/uhttpd/patches/0011-client-parse-chunked-transfer-chunk-size-safely.patch @@ -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; diff --git a/package/network/services/uhttpd/patches/0012-auth-do-not-accept-stored-crypt-hash-as-plaintext-pa.patch b/package/network/services/uhttpd/patches/0012-auth-do-not-accept-stored-crypt-hash-as-plaintext-pa.patch new file mode 100644 index 0000000000..ddf77e7203 --- /dev/null +++ b/package/network/services/uhttpd/patches/0012-auth-do-not-accept-stored-crypt-hash-as-plaintext-pa.patch @@ -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; diff --git a/package/network/services/uhttpd/patches/0013-auth-replace-strcmp-with-constant-time-password-comp.patch b/package/network/services/uhttpd/patches/0013-auth-replace-strcmp-with-constant-time-password-comp.patch new file mode 100644 index 0000000000..a2131ed56f --- /dev/null +++ b/package/network/services/uhttpd/patches/0013-auth-replace-strcmp-with-constant-time-password-comp.patch @@ -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; + diff --git a/package/network/services/uhttpd/patches/0014-auth-classify-p-lookups-by-account-state.patch b/package/network/services/uhttpd/patches/0014-auth-classify-p-lookups-by-account-state.patch new file mode 100644 index 0000000000..e46ac69227 --- /dev/null +++ b/package/network/services/uhttpd/patches/0014-auth-classify-p-lookups-by-account-state.patch @@ -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; + } diff --git a/package/network/services/uhttpd/patches/0015-client-reject-unhandled-Transfer-Encoding-values.patch b/package/network/services/uhttpd/patches/0015-client-reject-unhandled-Transfer-Encoding-values.patch new file mode 100644 index 0000000000..96c0ffb4b0 --- /dev/null +++ b/package/network/services/uhttpd/patches/0015-client-reject-unhandled-Transfer-Encoding-values.patch @@ -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; diff --git a/package/network/services/uhttpd/patches/0016-client-close-connection-on-invalid-chunk-length.patch b/package/network/services/uhttpd/patches/0016-client-close-connection-on-invalid-chunk-length.patch new file mode 100644 index 0000000000..15b92ab15f --- /dev/null +++ b/package/network/services/uhttpd/patches/0016-client-close-connection-on-invalid-chunk-length.patch @@ -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; + } + diff --git a/package/network/services/uhttpd/patches/0017-ubus-close-connection-on-POST-body-parse-error.patch b/package/network/services/uhttpd/patches/0017-ubus-close-connection-on-POST-body-parse-error.patch new file mode 100644 index 0000000000..9274972af0 --- /dev/null +++ b/package/network/services/uhttpd/patches/0017-ubus-close-connection-on-POST-body-parse-error.patch @@ -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; + } |