CVE-2021-36221

Origin: https://github.com/golang/go/commit/ba93baa74a52d57ae79313313ea990cc791ef50e
Reviewed-by: Sylvain Beucler <beuc@debian.org>
Last-Update: 2023-04-15

From ba93baa74a52d57ae79313313ea990cc791ef50e Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Wed, 7 Jul 2021 16:34:34 -0700
Subject: [PATCH] [release-branch.go1.15] net/http/httputil: close incoming
 ReverseProxy request body

Reading from an incoming request body after the request handler aborts
with a panic can cause a panic, becuse http.Server does not (contrary
to its documentation) close the request body in this case.

Always close the incoming request body in ReverseProxy.ServeHTTP to
ensure that any in-flight outgoing requests using the body do not
read from it.

Fixes #47473
Updates #46866
Fixes CVE-2021-36221

Change-Id: I310df269200ad8732c5d9f1a2b00de68725831df
Reviewed-on: https://go-review.googlesource.com/c/go/+/333191
Trust: Damien Neil <dneil@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
(cherry picked from commit b7a85e0003cedb1b48a1fd3ae5b746ec6330102e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/338550
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Gbp-Pq: Name CVE-2021-36221.patch

diff --git a/src/net/http/httputil/reverseproxy.go b/src/net/http/httputil/reverseproxy.go
index 1dddaa95a7707c134bcdd07ec6c385b1c5dfec81..92a1fff10bf8a4723a6a92479200bfe47d8e5061 100644 (file)
--- a/src/net/http/httputil/reverseproxy.go
+++ b/src/net/http/httputil/reverseproxy.go
@@ -191,7 +191,15 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
        if req.ContentLength == 0 {
                outreq.Body = nil // Issue 16036: nil Body for http.Transport retries
        }
-
+       if outreq.Body != nil {
+               // Reading from the request body after returning from a handler is not
+               // allowed, and the RoundTrip goroutine that reads the Body can outlive
+               // this handler. This can lead to a crash if the handler panics (see
+               // Issue 46866). Although calling Close doesn't guarantee there isn't
+               // any Read in flight after the handle returns, in practice it's safe to
+               // read after closing it.
+               defer outreq.Body.Close()
+       }
        outreq.Header = cloneHeader(req.Header)
 
        p.Director(outreq)

diff --git a/src/net/http/httputil/reverseproxy_test.go b/src/net/http/httputil/reverseproxy_test.go
index 2f75b4e34ec9917356c0a11806446fd0f99962f2..a6470b71d73ef34eac9fe11c18cae1f0a5de6a25 100644 (file)
--- a/src/net/http/httputil/reverseproxy_test.go
+++ b/src/net/http/httputil/reverseproxy_test.go
@@ -946,3 +946,42 @@ func TestReverseProxy_PanicBodyError(t *testing.T) {
        req, _ := http.NewRequest("GET", "http://foo.tld/", nil)
        rproxy.ServeHTTP(httptest.NewRecorder(), req)
 }
+
+// Issue #46866: panic without closing incoming request body causes a panic
+func TestReverseProxy_PanicClosesIncomingBody(t *testing.T) {
+       backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+               out := "this call was relayed by the reverse proxy"
+               // Coerce a wrong content length to induce io.ErrUnexpectedEOF
+               w.Header().Set("Content-Length", fmt.Sprintf("%d", len(out)*2))
+               fmt.Fprintln(w, out)
+       }))
+       defer backend.Close()
+       backendURL, err := url.Parse(backend.URL)
+       if err != nil {
+               t.Fatal(err)
+       }
+       proxyHandler := NewSingleHostReverseProxy(backendURL)
+       proxyHandler.ErrorLog = log.New(ioutil.Discard, "", 0) // quiet for tests
+       frontend := httptest.NewServer(proxyHandler)
+       defer frontend.Close()
+       frontendClient := frontend.Client()
+
+       var wg sync.WaitGroup
+       for i := 0; i < 2; i++ {
+               wg.Add(1)
+               go func() {
+                       defer wg.Done()
+                       for j := 0; j < 10; j++ {
+                               const reqLen = 6 * 1024 * 1024
+                               req, _ := http.NewRequest("POST", frontend.URL, &io.LimitedReader{R: neverEnding('x'), N: reqLen})
+                               req.ContentLength = reqLen
+                               resp, _ := frontendClient.Transport.RoundTrip(req)
+                               if resp != nil {
+                                       io.Copy(ioutil.Discard, resp.Body)
+                                       resp.Body.Close()
+                               }
+                       }
+               }()
+       }
+       wg.Wait()
+}