Mailing List Archive

commit 9cb03fe7e5a98a7aecdb6d11d6656bb3c7a509e4
Author: Poul-Henning Kamp <phk@FreeBSD.org>
Date: Tue Aug 22 08:41:21 2023 +0000

Redo H2 field validation

Fixes #3952

diff --git a/bin/varnishd/http2/cache_http2_hpack.c b/bin/varnishd/http2/cache_http2_hpack.c
index 57ca82390..016e78c11 100644
--- a/bin/varnishd/http2/cache_http2_hpack.c
+++ b/bin/varnishd/http2/cache_http2_hpack.c
@@ -37,6 +37,7 @@
#include "http2/cache_http2.h"
#include "vct.h"

+// rfc9113,l,2493,2528
static h2_error
h2h_checkhdr(const struct http *hp, const char *b, size_t namelen, size_t len)
{
@@ -46,46 +47,80 @@ h2h_checkhdr(const struct http *hp, const char *b, size_t namelen, size_t len)
AN(b);
assert(namelen >= 2); /* 2 chars from the ': ' that we added */
assert(namelen <= len);
+ assert(b[namelen - 2] == ':');
+ assert(b[namelen - 1] == ' ');

if (namelen == 2) {
VSLb(hp->vsl, SLT_BogoHeader, "Empty name");
return (H2SE_PROTOCOL_ERROR);
}

- for (p = b; p < b + len; p++) {
- if (p < b + (namelen - 2)) {
- /* Check valid name characters */
- if (p == b && *p == ':')
- continue; /* pseudo-header */
+ // VSLb(hp->vsl, SLT_Debug, "CHDR [%.*s] [%.*s]",
+ // (int)namelen, b, (int)(len - namelen), b + namelen);
+
+ int state = 0;
+ for (p = b; p < b + namelen - 2; p++) {
+ switch(state) {
+ case 0: /* First char of field */
+ state = 1;
+ if (*p == ':')
+ break;
+ /* FALL_THROUGH */
+ case 1: /* field name */
+ if (*p <= 0x20 || *p >= 0x7f) {
+ VSLb(hp->vsl, SLT_BogoHeader,
+ "Illegal field header name (control): %.*s",
+ (int)(len > 20 ? 20 : len), b);
+ return (H2SE_PROTOCOL_ERROR);
+ }
if (isupper(*p)) {
VSLb(hp->vsl, SLT_BogoHeader,
- "Illegal header name (upper-case): %.*s",
+ "Illegal field header name (upper-case): %.*s",
(int)(len > 20 ? 20 : len), b);
return (H2SE_PROTOCOL_ERROR);
}
- if (vct_istchar(*p)) {
- /* XXX: vct should have a proper class for
- this avoiding two checks */
- continue;
+ if (!vct_istchar(*p) || *p == ':') {
+ VSLb(hp->vsl, SLT_BogoHeader,
+ "Illegal field header name (non-token): %.*s",
+ (int)(len > 20 ? 20 : len), b);
+ return (H2SE_PROTOCOL_ERROR);
}
- VSLb(hp->vsl, SLT_BogoHeader,
- "Illegal header name: %.*s",
- (int)(len > 20 ? 20 : len), b);
- return (H2SE_PROTOCOL_ERROR);
- } else if (p < b + namelen) {
- /* ': ' added by us */
- assert(*p == ':' || *p == ' ');
- } else {
- /* Check valid value characters */
- if (!vct_isctl(*p) || vct_issp(*p))
- continue;
- VSLb(hp->vsl, SLT_BogoHeader,
- "Illegal header value: %.*s",
- (int)(len > 20 ? 20 : len), b);
- return (H2SE_PROTOCOL_ERROR);
+ break;
+ default:
+ WRONG("http2 field name validation state");
}
}

+ state = 2;
+ for (p = b + namelen; p < b + len; p++) {
+ switch(state) {
+ case 2: /* First char of field */
+ if (*p == ' ' || *p == 0x09) {
+ VSLb(hp->vsl, SLT_BogoHeader,
+ "Illegal field value start %.*s",
+ (int)(len > 20 ? 20 : len), b);
+ return (H2SE_PROTOCOL_ERROR);
+ }
+ state = 3;
+ /* FALL_THROUGH */
+ case 3: /* field value character */
+ if (*p != 0x09 && (*p < 0x20 || *p == 0x7f)) {
+ VSLb(hp->vsl, SLT_BogoHeader,
+ "Illegal field value (control) %.*s",
+ (int)(len > 20 ? 20 : len), b);
+ return (H2SE_PROTOCOL_ERROR);
+ }
+ break;
+ default:
+ WRONG("http2 field value validation state");
+ }
+ }
+ if (state == 3 && b[len - 1] <= 0x20) {
+ VSLb(hp->vsl, SLT_BogoHeader,
+ "Illegal val (end) %.*s",
+ (int)(len > 20 ? 20 : len), b);
+ return (H2SE_PROTOCOL_ERROR);
+ }
return (0);
}

@@ -281,6 +316,7 @@ h2h_decode_fini(const struct h2_sess *h2)
* block. This is a connection level error.
*
* H2E_PROTOCOL_ERROR: Malformed header or duplicate pseudo-header.
+ * Violation of field name/value charsets
*/
h2_error
h2h_decode_bytes(struct h2_sess *h2, const uint8_t *in, size_t in_l)
diff --git a/bin/varnishtest/tests/t02023.vtc b/bin/varnishtest/tests/t02023.vtc
index cfd843da3..59c7fe5d7 100644
--- a/bin/varnishtest/tests/t02023.vtc
+++ b/bin/varnishtest/tests/t02023.vtc
@@ -46,3 +46,47 @@ client c1 {
rxrst
} -run
} -run
+
+varnish v1 -vsl_catchup
+
+client c1 {
+ stream 1 {
+ txreq -hdr "fo o" " bar"
+ rxrst
+ } -run
+} -run
+
+client c1 {
+ stream 1 {
+ txreq -hdr "foo" " "
+ rxrst
+ } -run
+} -run
+
+client c1 {
+ stream 1 {
+ txreq -hdr ":foo" "bar"
+ rxrst
+ } -run
+} -run
+
+client c1 {
+ stream 1 {
+ txreq -hdr "foo" "b\x0car"
+ rxrst
+ } -run
+} -run
+
+client c1 {
+ stream 1 {
+ txreq -hdr "f o" "bar"
+ rxrst
+ } -run
+} -run
+
+client c1 {
+ stream 1 {
+ txreq -hdr "f: o" "bar"
+ rxrst
+ } -run
+} -run
_______________________________________________
varnish-commit mailing list
varnish-commit@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit