Skip to content

Commit

Permalink
go: fix CVE-2023-29406 net/http: insufficient sanitization of Host he…
Browse files Browse the repository at this point in the history 
…ader

Source: poky
MR: 127621, 127432
Type: Security Fix
Disposition: Merged from poky
ChangeID: f7f7d5514cf0f63a34d1c75efb031829808ddf2e
Description:

(From OE-Core rev: 07e03175de91739064ae5530b3df093b4d05510b)

Signed-off-by: Vivek Kumbhar <[email protected]>
Signed-off-by: Steve Sakoman <[email protected]>
Signed-off-by: Jeremy A. Puhlman <[email protected]>
  • Loading branch information
@VKumbhar24 @jpuhlman
VKumbhar24 authored and jpuhlman committed Aug 23, 2023
1 parent d4b2bd7 commit 27e7fe9
Show file tree
Hide file tree
Showing 2 changed files with 213 additions and 0 deletions.
1 change: 1 addition & 0 deletions meta/recipes-devtools/go/go-1.14.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ SRC_URI += "\
file://CVE-2023-29402.patch \
file://CVE-2023-29404.patch \
file://CVE-2023-29400.patch \
file://CVE-2023-29406.patch \
"

SRC_URI_append_libc-musl = " file://0009-ld-replace-glibc-dynamic-linker-with-musl.patch"
Expand Down
212 changes: 212 additions & 0 deletions meta/recipes-devtools/go/go-1.14/CVE-2023-29406.patch
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,212 @@
From 5fa6923b1ea891400153d04ddf1545e23b40041b Mon Sep 17 00:00:00 2001
From: Damien Neil <[email protected]>
Date: Wed, 28 Jun 2023 13:20:08 -0700
Subject: [PATCH] [release-branch.go1.19] net/http: validate Host header before
sending

Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.

Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.

Updates #60374
Fixes #61075
For CVE-2023-29406

Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996
Reviewed-by: Tatiana Bradley <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Damien Neil <[email protected]>
(cherry picked from commit 499458f7ca04087958987a33c2703c3ef03e27e2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/507358
Run-TryBot: Tatiana Bradley <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>

Upstream-Status: Backport [https://github.com/golang/go/commit/5fa6923b1ea891400153d04ddf1545e23b40041b]
CVE: CVE-2023-29406
Signed-off-by: Vivek Kumbhar <[email protected]>
---
src/net/http/http_test.go | 29 ---------------------
src/net/http/request.go | 47 ++++++++--------------------------
src/net/http/request_test.go | 11 ++------
src/net/http/transport_test.go | 18 +++++++++++++
4 files changed, 31 insertions(+), 74 deletions(-)

diff --git a/src/net/http/http_test.go b/src/net/http/http_test.go
index f4ea52d..ea38cb4 100644
--- a/src/net/http/http_test.go
+++ b/src/net/http/http_test.go
@@ -49,35 +49,6 @@ func TestForeachHeaderElement(t *testing.T) {
}
}

-func TestCleanHost(t *testing.T) {
- tests := []struct {
- in, want string
- }{
- {"www.google.com", "www.google.com"},
- {"www.google.com foo", "www.google.com"},
- {"www.google.com/foo", "www.google.com"},
- {" first character is a space", ""},
- {"[1::6]:8080", "[1::6]:8080"},
-
- // Punycode:
- {"гофер.рф/foo", "xn--c1ae0ajs.xn--p1ai"},
- {"bücher.de", "xn--bcher-kva.de"},
- {"bücher.de:8080", "xn--bcher-kva.de:8080"},
- // Verify we convert to lowercase before punycode:
- {"BÜCHER.de", "xn--bcher-kva.de"},
- {"BÜCHER.de:8080", "xn--bcher-kva.de:8080"},
- // Verify we normalize to NFC before punycode:
- {"gophér.nfc", "xn--gophr-esa.nfc"}, // NFC input; no work needed
- {"goph\u0065\u0301r.nfd", "xn--gophr-esa.nfd"}, // NFD input
- }
- for _, tt := range tests {
- got := cleanHost(tt.in)
- if tt.want != got {
- t.Errorf("cleanHost(%q) = %q, want %q", tt.in, got, tt.want)
- }
- }
-}
-
// Test that cmd/go doesn't link in the HTTP server.
//
// This catches accidental dependencies between the HTTP transport and
diff --git a/src/net/http/request.go b/src/net/http/request.go
index cb2edd2..2706300 100644
--- a/src/net/http/request.go
+++ b/src/net/http/request.go
@@ -18,7 +18,6 @@ import (
"io/ioutil"
"mime"
"mime/multipart"
- "net"
"net/http/httptrace"
"net/textproto"
"net/url"
@@ -26,7 +25,8 @@ import (
"strconv"
"strings"
"sync"
-
+
+ "golang.org/x/net/http/httpguts"
"golang.org/x/net/idna"
)

@@ -557,12 +557,19 @@ func (r *Request) write(w io.Writer, usingProxy bool, extraHeaders Header, waitF
// is not given, use the host from the request URL.
//
// Clean the host, in case it arrives with unexpected stuff in it.
- host := cleanHost(r.Host)
+ host := r.Host
if host == "" {
if r.URL == nil {
return errMissingHost
}
- host = cleanHost(r.URL.Host)
+ host = r.URL.Host
+ }
+ host, err = httpguts.PunycodeHostPort(host)
+ if err != nil {
+ return err
+ }
+ if !httpguts.ValidHostHeader(host) {
+ return errors.New("http: invalid Host header")
}

// According to RFC 6874, an HTTP client, proxy, or other
@@ -717,38 +724,6 @@ func idnaASCII(v string) (string, error) {
return idna.Lookup.ToASCII(v)
}

-// cleanHost cleans up the host sent in request's Host header.
-//
-// It both strips anything after '/' or ' ', and puts the value
-// into Punycode form, if necessary.
-//
-// Ideally we'd clean the Host header according to the spec:
-// https://tools.ietf.org/html/rfc7230#section-5.4 (Host = uri-host [ ":" port ]")
-// https://tools.ietf.org/html/rfc7230#section-2.7 (uri-host -> rfc3986's host)
-// https://tools.ietf.org/html/rfc3986#section-3.2.2 (definition of host)
-// But practically, what we are trying to avoid is the situation in
-// issue 11206, where a malformed Host header used in the proxy context
-// would create a bad request. So it is enough to just truncate at the
-// first offending character.
-func cleanHost(in string) string {
- if i := strings.IndexAny(in, " /"); i != -1 {
- in = in[:i]
- }
- host, port, err := net.SplitHostPort(in)
- if err != nil { // input was just a host
- a, err := idnaASCII(in)
- if err != nil {
- return in // garbage in, garbage out
- }
- return a
- }
- a, err := idnaASCII(host)
- if err != nil {
- return in // garbage in, garbage out
- }
- return net.JoinHostPort(a, port)
-}
-
// removeZone removes IPv6 zone identifier from host.
// E.g., "[fe80::1%en0]:8080" to "[fe80::1]:8080"
func removeZone(host string) string {
diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go
index 461d66e..0d417ff 100644
--- a/src/net/http/request_test.go
+++ b/src/net/http/request_test.go
@@ -676,15 +676,8 @@ func TestRequestBadHost(t *testing.T) {
}
req.Host = "foo.com with spaces"
req.URL.Host = "foo.com with spaces"
- req.Write(logWrites{t, &got})
- want := []string{
- "GET /after HTTP/1.1\r\n",
- "Host: foo.com\r\n",
- "User-Agent: " + DefaultUserAgent + "\r\n",
- "\r\n",
- }
- if !reflect.DeepEqual(got, want) {
- t.Errorf("Writes = %q\n Want = %q", got, want)
+ if err := req.Write(logWrites{t, &got}); err == nil {
+ t.Errorf("Writing request with invalid Host: succeded, want error")
}
}

diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index fa0c370..0afb6b9 100644
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -6249,3 +6249,21 @@ func TestIssue32441(t *testing.T) {
t.Error(err)
}
}
+
+func TestRequestSanitization(t *testing.T) {
+ setParallel(t)
+ defer afterTest(t)
+
+ ts := newClientServerTest(t, h1Mode, HandlerFunc(func(rw ResponseWriter, req *Request) {
+ if h, ok := req.Header["X-Evil"]; ok {
+ t.Errorf("request has X-Evil header: %q", h)
+ }
+ })).ts
+ defer ts.Close()
+ req, _ := NewRequest("GET", ts.URL, nil)
+ req.Host = "go.dev\r\nX-Evil:evil"
+ resp, _ := ts.Client().Do(req)
+ if resp != nil {
+ resp.Body.Close()
+ }
+}
--
2.25.1

0 comments on commit 27e7fe9

Please sign in to comment.