| From 753e3f8da191c2ac400407d83c70f46900769417 Mon Sep 17 00:00:00 2001 |
| From: Hitendra Prajapati <hprajapati@mvista.com> |
| Date: Thu, 27 Oct 2022 12:22:41 +0530 |
| Subject: [PATCH] CVE-2022-2880 |
| |
| Upstream-Status: Backport [https://github.com/golang/go/commit/9d2c73a9fd69e45876509bb3bdb2af99bf77da1e] |
| CVE: CVE-2022-2880 |
| Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com> |
| |
| net/http/httputil: avoid query parameter |
| |
| Query parameter smuggling occurs when a proxy's interpretation |
| of query parameters differs from that of a downstream server. |
| Change ReverseProxy to avoid forwarding ignored query parameters. |
| |
| Remove unparsable query parameters from the outbound request |
| |
| * if req.Form != nil after calling ReverseProxy.Director; and |
| * before calling ReverseProxy.Rewrite. |
| |
| This change preserves the existing behavior of forwarding the |
| raw query untouched if a Director hook does not parse the query |
| by calling Request.ParseForm (possibly indirectly). |
| --- |
| src/net/http/httputil/reverseproxy.go | 36 +++++++++++ |
| src/net/http/httputil/reverseproxy_test.go | 74 ++++++++++++++++++++++ |
| 2 files changed, 110 insertions(+) |
| |
| diff --git a/src/net/http/httputil/reverseproxy.go b/src/net/http/httputil/reverseproxy.go |
| index 2072a5f..c6fb873 100644 |
| --- a/src/net/http/httputil/reverseproxy.go |
| +++ b/src/net/http/httputil/reverseproxy.go |
| @@ -212,6 +212,9 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) { |
| } |
| |
| p.Director(outreq) |
| + if outreq.Form != nil { |
| + outreq.URL.RawQuery = cleanQueryParams(outreq.URL.RawQuery) |
| + } |
| outreq.Close = false |
| |
| reqUpType := upgradeType(outreq.Header) |
| @@ -561,3 +564,36 @@ func (c switchProtocolCopier) copyToBackend(errc chan<- error) { |
| _, err := io.Copy(c.backend, c.user) |
| errc <- err |
| } |
| + |
| +func cleanQueryParams(s string) string { |
| + reencode := func(s string) string { |
| + v, _ := url.ParseQuery(s) |
| + return v.Encode() |
| + } |
| + for i := 0; i < len(s); { |
| + switch s[i] { |
| + case ';': |
| + return reencode(s) |
| + case '%': |
| + if i+2 >= len(s) || !ishex(s[i+1]) || !ishex(s[i+2]) { |
| + return reencode(s) |
| + } |
| + i += 3 |
| + default: |
| + i++ |
| + } |
| + } |
| + return s |
| +} |
| + |
| +func ishex(c byte) bool { |
| + switch { |
| + case '0' <= c && c <= '9': |
| + return true |
| + case 'a' <= c && c <= 'f': |
| + return true |
| + case 'A' <= c && c <= 'F': |
| + return true |
| + } |
| + return false |
| +} |
| diff --git a/src/net/http/httputil/reverseproxy_test.go b/src/net/http/httputil/reverseproxy_test.go |
| index 9a7223a..bc87a3b 100644 |
| --- a/src/net/http/httputil/reverseproxy_test.go |
| +++ b/src/net/http/httputil/reverseproxy_test.go |
| @@ -1269,3 +1269,77 @@ func TestSingleJoinSlash(t *testing.T) { |
| } |
| } |
| } |
| + |
| +const ( |
| + testWantsCleanQuery = true |
| + testWantsRawQuery = false |
| +) |
| + |
| +func TestReverseProxyQueryParameterSmugglingDirectorDoesNotParseForm(t *testing.T) { |
| + testReverseProxyQueryParameterSmuggling(t, testWantsRawQuery, func(u *url.URL) *ReverseProxy { |
| + proxyHandler := NewSingleHostReverseProxy(u) |
| + oldDirector := proxyHandler.Director |
| + proxyHandler.Director = func(r *http.Request) { |
| + oldDirector(r) |
| + } |
| + return proxyHandler |
| + }) |
| +} |
| + |
| +func TestReverseProxyQueryParameterSmugglingDirectorParsesForm(t *testing.T) { |
| + testReverseProxyQueryParameterSmuggling(t, testWantsCleanQuery, func(u *url.URL) *ReverseProxy { |
| + proxyHandler := NewSingleHostReverseProxy(u) |
| + oldDirector := proxyHandler.Director |
| + proxyHandler.Director = func(r *http.Request) { |
| + // Parsing the form causes ReverseProxy to remove unparsable |
| + // query parameters before forwarding. |
| + r.FormValue("a") |
| + oldDirector(r) |
| + } |
| + return proxyHandler |
| + }) |
| +} |
| + |
| +func testReverseProxyQueryParameterSmuggling(t *testing.T, wantCleanQuery bool, newProxy func(*url.URL) *ReverseProxy) { |
| + const content = "response_content" |
| + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + w.Write([]byte(r.URL.RawQuery)) |
| + })) |
| + defer backend.Close() |
| + backendURL, err := url.Parse(backend.URL) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + proxyHandler := newProxy(backendURL) |
| + frontend := httptest.NewServer(proxyHandler) |
| + defer frontend.Close() |
| + |
| + // Don't spam output with logs of queries containing semicolons. |
| + backend.Config.ErrorLog = log.New(io.Discard, "", 0) |
| + frontend.Config.ErrorLog = log.New(io.Discard, "", 0) |
| + |
| + for _, test := range []struct { |
| + rawQuery string |
| + cleanQuery string |
| + }{{ |
| + rawQuery: "a=1&a=2;b=3", |
| + cleanQuery: "a=1", |
| + }, { |
| + rawQuery: "a=1&a=%zz&b=3", |
| + cleanQuery: "a=1&b=3", |
| + }} { |
| + res, err := frontend.Client().Get(frontend.URL + "?" + test.rawQuery) |
| + if err != nil { |
| + t.Fatalf("Get: %v", err) |
| + } |
| + defer res.Body.Close() |
| + body, _ := io.ReadAll(res.Body) |
| + wantQuery := test.rawQuery |
| + if wantCleanQuery { |
| + wantQuery = test.cleanQuery |
| + } |
| + if got, want := string(body), wantQuery; got != want { |
| + t.Errorf("proxy forwarded raw query %q as %q, want %q", test.rawQuery, got, want) |
| + } |
| + } |
| +} |
| -- |
| 2.25.1 |
| |