From 7b273dc5907558980fced5f0cd4eff622506c478 Mon Sep 17 00:00:00 2001 From: Erik Dubbelboer Date: Tue, 11 Jun 2024 08:41:16 +0200 Subject: [PATCH] Don't allow \r in header names (#1789) * Don't allow \r in header names From RFC 9112: A sender MUST NOT generate a bare CR (a CR character not immediately followed by LF) within any protocol elements other than the content. A recipient of such a bare CR MUST consider that element to be invalid or replace each bare CR with SP before processing the element or forwarding the message. net/http seems to completely error on this, so let's do the same. Fixes https://github.com/valyala/fasthttp/issues/1785 * Validate the full header field --- bytesconv_table.go | 2 +- bytesconv_table_gen.go | 4 ++-- header.go | 23 +++++++++++++++++------ header_test.go | 13 +++++++++++++ 4 files changed, 33 insertions(+), 9 deletions(-) diff --git a/bytesconv_table.go b/bytesconv_table.go index 2224535401..5b230f1a5d 100644 --- a/bytesconv_table.go +++ b/bytesconv_table.go @@ -8,4 +8,4 @@ const toLowerTable = "\x00\x01\x02\x03\x04\x05\x06\a\b\t\n\v\f\r\x0e\x0f\x10\x11 const toUpperTable = "\x00\x01\x02\x03\x04\x05\x06\a\b\t\n\v\f\r\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`ABCDEFGHIJKLMNOPQRSTUVWXYZ{|}~\x7f\x80\x81\x82\x83\x84\x85\x86\x87\x88\x89\x8a\x8b\x8c\x8d\x8e\x8f\x90\x91\x92\x93\x94\x95\x96\x97\x98\x99\x9a\x9b\x9c\x9d\x9e\x9f\xa0\xa1\xa2\xa3\xa4\xa5\xa6\xa7\xa8\xa9\xaa\xab\xac\xad\xae\xaf\xb0\xb1\xb2\xb3\xb4\xb5\xb6\xb7\xb8\xb9\xba\xbb\xbc\xbd\xbe\xbf\xc0\xc1\xc2\xc3\xc4\xc5\xc6\xc7\xc8\xc9\xca\xcb\xcc\xcd\xce\xcf\xd0\xd1\xd2\xd3\xd4\xd5\xd6\xd7\xd8\xd9\xda\xdb\xdc\xdd\xde\xdf\xe0\xe1\xe2\xe3\xe4\xe5\xe6\xe7\xe8\xe9\xea\xeb\xec\xed\xee\xef\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff" const quotedArgShouldEscapeTable = "\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x01\x01\x01\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x01\x01\x01\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x01\x01\x00\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01" const quotedPathShouldEscapeTable = "\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x00\x01\x00\x01\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x01\x01\x01\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x01\x01\x00\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01" -const tcharTable = "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x01\x01\x01\x01\x01\x00\x00\x01\x01\x00\x01\x01\x00\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x00\x00\x00\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x00\x01\x00\x01\x00" +const validHeaderFieldByteTable = "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x01\x01\x01\x01\x01\x00\x00\x01\x01\x00\x01\x01\x00\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x00\x00\x00\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x00\x01\x00\x01\x00" diff --git a/bytesconv_table_gen.go b/bytesconv_table_gen.go index 496aaabdd7..4fa8c16cdd 100644 --- a/bytesconv_table_gen.go +++ b/bytesconv_table_gen.go @@ -98,7 +98,7 @@ func main() { return a }() - tcharTable := func() [128]byte { + validHeaderFieldByteTable := func() [128]byte { // Should match net/textproto's validHeaderFieldByte(c byte) bool // Defined by RFC 9110 5.6.2 // tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." / @@ -125,7 +125,7 @@ func main() { fmt.Fprintf(w, "const toUpperTable = %q\n", toUpperTable) fmt.Fprintf(w, "const quotedArgShouldEscapeTable = %q\n", quotedArgShouldEscapeTable) fmt.Fprintf(w, "const quotedPathShouldEscapeTable = %q\n", quotedPathShouldEscapeTable) - fmt.Fprintf(w, "const tcharTable = %q\n", tcharTable) + fmt.Fprintf(w, "const validHeaderFieldByteTable = %q\n", validHeaderFieldByteTable) if err := os.WriteFile("bytesconv_table.go", w.Bytes(), 0o660); err != nil { log.Fatal(err) diff --git a/header.go b/header.go index a5b42e9ed0..b02629447e 100644 --- a/header.go +++ b/header.go @@ -548,7 +548,7 @@ func (h *ResponseHeader) AddTrailerBytes(trailer []byte) error { // validHeaderFieldByte returns true if c is a valid tchar as defined // by section 5.6.2 of [RFC9110]. func validHeaderFieldByte(c byte) bool { - return c < 128 && tcharTable[c] == 1 + return c < 128 && validHeaderFieldByteTable[c] == 1 } // VisitHeaderParams calls f for each parameter in the given header bytes. @@ -2947,8 +2947,17 @@ func (h *ResponseHeader) parseHeaders(buf []byte) (int, error) { s.disableNormalizing = h.disableNormalizing var err error var kv *argsKV + +outer: for s.next() { if len(s.key) > 0 { + for _, ch := range s.key { + if !validHeaderFieldByte(ch) { + err = fmt.Errorf("invalid header key %q", s.key) + continue outer + } + } + switch s.key[0] | 0x20 { case 'c': if caseInsensitiveCompare(s.key, strContentType) { @@ -3035,13 +3044,15 @@ func (h *RequestHeader) parseHeaders(buf []byte) (int, error) { s.b = buf s.disableNormalizing = h.disableNormalizing var err error + +outer: for s.next() { if len(s.key) > 0 { - // Spaces between the header key and colon are not allowed. - // See RFC 7230, Section 3.2.4. - if bytes.IndexByte(s.key, ' ') != -1 || bytes.IndexByte(s.key, '\t') != -1 { - err = fmt.Errorf("invalid header key %q", s.key) - continue + for _, ch := range s.key { + if !validHeaderFieldByte(ch) { + err = fmt.Errorf("invalid header key %q", s.key) + continue outer + } } if h.disableSpecialHeader { diff --git a/header_test.go b/header_test.go index 7e30162da2..15f4918873 100644 --- a/header_test.go +++ b/header_test.go @@ -154,6 +154,19 @@ func TestResponseHeaderMultiLinePanicked(t *testing.T) { } } +func TestRequestHeaderLooseBackslashR(t *testing.T) { + t.Parallel() + + s := "GET / HTTP/1.1\r\n" + + "Host: go.dev\r\n" + + "\rFoo: bar\r\n" + + "\r\n" + header := new(RequestHeader) + if _, err := header.parse([]byte(s)); err == nil { + t.Fatal("expected error, got ") + } +} + func TestResponseHeaderEmptyValueFromHeader(t *testing.T) { t.Parallel()