Hi Ben, Rajat,
On Sun, Jan 18, 2026 at 08:49:04PM +0000, Ben Kallus wrote:
> Haven't reviewed, this, but wanted to chime in with a correction:
>
> > A quoted-string continues until the closing DQUOTE, even if it contains
> `\r\n` sequences.
>
> This isn't true. An RFC-compliant parser should reject \r\n within a
> chunk-ext and respond 400. Go look at the definitions of qdtext and
> quoted-pair; neither one allows CR or LF.
>
> If nginx indeed behaves the way you claim, this is also a bug in nginx.
So I've rechecked the various RFCs and I agree with you:
RFC9112/7.1.1:
chunk-ext = *( BWS ";" BWS chunk-ext-name [ BWS "=" BWS chunk-ext-val ] )
chunk-ext-name = token
chunk-ext-val = token / quoted-string
RFC9110:
quoted-string = DQUOTE *( qdtext / quoted-pair ) DQUOTE
qdtext = HTAB / SP / %x21 / %x23-5B / %x5D-7E / obs-text
quoted-pair = "\" ( HTAB / SP / VCHAR / obs-text )
obs-text = %x80-FF
VCHAR = any visible US-ASCII character (i.e. %x21-7E)
"A recipient MUST ignore unrecognized chunk extensions."
So here since we use no such extension, we just ignore them all, which
also means that we don't implement code to validate that those we're
supposed to ignore have correctly paired quotes, since CR/LF are forbidden
within these quotes anyway.
The chunk size parser tries to follow these rules as closely as possible
(while correctly ignoring the extensions):
h1_parse_chunk_size():
/* The chunk size is in the following form, though we are only
* interested in the size and CRLF :
* 1*HEXDIGIT *WSP *[ ';' extensions ] CRLF
*/
(...)
if (likely(*ptr == ';')) {
/* chunk extension, ends at next CRLF */
(...)
Now we can see what it would cost to check for unmatched quotes in the
extension anyway. I gave it a try with the attached patch. Could you
please check if it suffices to protect the server ? I think it could
reasonably be backported if it helps. However I'm not seeing that as
a bug given that we follow the spec.
Rajat, if you want to share your PoC (even though I understand how it
works), you can send it to security at haproxy.org.
Thanks,
Willy
>From 62589ad10044d172d90cee01efdd39d837a2cd24 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <[email protected]>
Date: Mon, 19 Jan 2026 08:42:32 +0100
Subject: WIP: h1: still validate extensions that we ignore
---
include/haproxy/h1.h | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/include/haproxy/h1.h b/include/haproxy/h1.h
index a01b53855d..22176a935e 100644
--- a/include/haproxy/h1.h
+++ b/include/haproxy/h1.h
@@ -263,6 +263,8 @@ static inline int h1_parse_chunk_size(const struct buffer
*buf, int start, int s
const char *ptr_old = ptr;
const char *end = b_wrap(buf);
uint64_t chunk = 0;
+ int quote = 0;
+ int backslash = 0;
stop -= start; // bytes left
start = stop; // bytes to transfer
@@ -321,19 +323,34 @@ static inline int h1_parse_chunk_size(const struct buffer
*buf, int start, int s
break;
}
else if (likely(*ptr == ';')) {
- /* chunk extension, ends at next CRLF */
+ /* chunk extension, ends at next CRLF. We still
+ * validate the parity of quotes if any.
+ */
if (++ptr >= end)
ptr = b_orig(buf);
if (--stop == 0)
return 0;
while (!HTTP_IS_CRLF(*ptr)) {
+ if (backslash)
+ backslash = 0;
+ else if (*ptr == '\\' && quote)
+ backslash = 1;
+ else if (*ptr == '\\')
+ goto error;
+ else if (*ptr == '"')
+ quote = !quote;
if (++ptr >= end)
ptr = b_orig(buf);
if (--stop == 0)
return 0;
}
/* we have a CRLF now, loop above */
+ /* for the sake of safety, explicitly reject CRLF
+ * inside quotes or after a backslash.
+ */
+ if (quote || backslash)
+ goto error;
continue;
}
else
--
2.35.3
