diff options
author | Andrew Bonventre <andybons@golang.org> | 2018-03-23 16:40:15 -0400 |
---|---|---|
committer | Andrew Bonventre <andybons@golang.org> | 2018-03-29 06:04:25 +0000 |
commit | 703bb6a7124da10fea81bf1e3318b37d8ebd4ec9 (patch) | |
tree | 6ae85c2ad7311bf18a1212c154425415c10c205c | |
parent | 15474dce9d295055c70c0a6ad092fd95563ddca9 (diff) | |
download | go-703bb6a7124da10fea81bf1e3318b37d8ebd4ec9.tar.gz go-703bb6a7124da10fea81bf1e3318b37d8ebd4ec9.zip |
[release-branch.go1.9] net/http/pprof: harden handler responses
A very small number of old browsers consider content as HTML
even when it is explicitly stated in the Content-Type header
that it is not. If content served is based on user-supplied
input, then an XSS is possible. Introduce three mitigations:
+ Don't reflect user input in error strings
+ Set a Content-Disposition header when requesting a resource
that should never be displayed in a browser window
+ Set X-Content-Type-Options: nosniff on all responses
Change-Id: I81c9d6736e0439ebd1db99cd7fb701cc56d24805
Reviewed-on: https://go-review.googlesource.com/102318
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/103164
Reviewed-by: Andrew Bonventre <andybons@golang.org>
-rw-r--r-- | src/net/http/pprof/pprof.go | 52 | ||||
-rw-r--r-- | src/net/http/pprof/pprof_test.go | 69 |
2 files changed, 98 insertions, 23 deletions
diff --git a/src/net/http/pprof/pprof.go b/src/net/http/pprof/pprof.go index 12c7599ab0..7f01ee47a0 100644 --- a/src/net/http/pprof/pprof.go +++ b/src/net/http/pprof/pprof.go @@ -80,6 +80,7 @@ func init() { // command line, with arguments separated by NUL bytes. // The package initialization registers it as /debug/pprof/cmdline. func Cmdline(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Content-Type-Options", "nosniff") w.Header().Set("Content-Type", "text/plain; charset=utf-8") fmt.Fprintf(w, strings.Join(os.Args, "\x00")) } @@ -100,33 +101,36 @@ func durationExceedsWriteTimeout(r *http.Request, seconds float64) bool { return ok && srv.WriteTimeout != 0 && seconds >= srv.WriteTimeout.Seconds() } +func serveError(w http.ResponseWriter, status int, txt string) { + w.Header().Set("Content-Type", "text/plain; charset=utf-8") + w.Header().Set("X-Go-Pprof", "1") + w.Header().Del("Content-Disposition") + w.WriteHeader(status) + fmt.Fprintln(w, txt) +} + // Profile responds with the pprof-formatted cpu profile. // The package initialization registers it as /debug/pprof/profile. func Profile(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Content-Type-Options", "nosniff") sec, _ := strconv.ParseInt(r.FormValue("seconds"), 10, 64) if sec == 0 { sec = 30 } if durationExceedsWriteTimeout(r, float64(sec)) { - w.Header().Set("Content-Type", "text/plain; charset=utf-8") - w.Header().Set("X-Go-Pprof", "1") - w.WriteHeader(http.StatusBadRequest) - fmt.Fprintln(w, "profile duration exceeds server's WriteTimeout") + serveError(w, http.StatusBadRequest, "profile duration exceeds server's WriteTimeout") return } // Set Content Type assuming StartCPUProfile will work, // because if it does it starts writing. w.Header().Set("Content-Type", "application/octet-stream") + w.Header().Set("Content-Disposition", `attachment; filename="profile"`) if err := pprof.StartCPUProfile(w); err != nil { // StartCPUProfile failed, so no writes yet. - // Can change header back to text content - // and send error code. - w.Header().Set("Content-Type", "text/plain; charset=utf-8") - w.Header().Set("X-Go-Pprof", "1") - w.WriteHeader(http.StatusInternalServerError) - fmt.Fprintf(w, "Could not enable CPU profiling: %s\n", err) + serveError(w, http.StatusInternalServerError, + fmt.Sprintf("Could not enable CPU profiling: %s", err)) return } sleep(w, time.Duration(sec)*time.Second) @@ -137,29 +141,25 @@ func Profile(w http.ResponseWriter, r *http.Request) { // Tracing lasts for duration specified in seconds GET parameter, or for 1 second if not specified. // The package initialization registers it as /debug/pprof/trace. func Trace(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Content-Type-Options", "nosniff") sec, err := strconv.ParseFloat(r.FormValue("seconds"), 64) if sec <= 0 || err != nil { sec = 1 } if durationExceedsWriteTimeout(r, sec) { - w.Header().Set("Content-Type", "text/plain; charset=utf-8") - w.Header().Set("X-Go-Pprof", "1") - w.WriteHeader(http.StatusBadRequest) - fmt.Fprintln(w, "profile duration exceeds server's WriteTimeout") + serveError(w, http.StatusBadRequest, "profile duration exceeds server's WriteTimeout") return } // Set Content Type assuming trace.Start will work, // because if it does it starts writing. w.Header().Set("Content-Type", "application/octet-stream") + w.Header().Set("Content-Disposition", `attachment; filename="trace"`) if err := trace.Start(w); err != nil { // trace.Start failed, so no writes yet. - // Can change header back to text content and send error code. - w.Header().Set("Content-Type", "text/plain; charset=utf-8") - w.Header().Set("X-Go-Pprof", "1") - w.WriteHeader(http.StatusInternalServerError) - fmt.Fprintf(w, "Could not enable tracing: %s\n", err) + serveError(w, http.StatusInternalServerError, + fmt.Sprintf("Could not enable tracing: %s", err)) return } sleep(w, time.Duration(sec*float64(time.Second))) @@ -170,6 +170,7 @@ func Trace(w http.ResponseWriter, r *http.Request) { // responding with a table mapping program counters to function names. // The package initialization registers it as /debug/pprof/symbol. func Symbol(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Content-Type-Options", "nosniff") w.Header().Set("Content-Type", "text/plain; charset=utf-8") // We have to read the whole POST body before @@ -222,18 +223,23 @@ func Handler(name string) http.Handler { type handler string func (name handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "text/plain; charset=utf-8") - debug, _ := strconv.Atoi(r.FormValue("debug")) + w.Header().Set("X-Content-Type-Options", "nosniff") p := pprof.Lookup(string(name)) if p == nil { - w.WriteHeader(404) - fmt.Fprintf(w, "Unknown profile: %s\n", name) + serveError(w, http.StatusNotFound, "Unknown profile") return } gc, _ := strconv.Atoi(r.FormValue("gc")) if name == "heap" && gc > 0 { runtime.GC() } + debug, _ := strconv.Atoi(r.FormValue("debug")) + if debug != 0 { + w.Header().Set("Content-Type", "text/plain; charset=utf-8") + } else { + w.Header().Set("Content-Type", "application/octet-stream") + w.Header().Set("Content-Disposition", fmt.Sprintf(`attachment; filename="%s"`, name)) + } p.WriteTo(w, debug) } diff --git a/src/net/http/pprof/pprof_test.go b/src/net/http/pprof/pprof_test.go new file mode 100644 index 0000000000..47dd35b9b0 --- /dev/null +++ b/src/net/http/pprof/pprof_test.go @@ -0,0 +1,69 @@ +// Copyright 2018 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package pprof + +import ( + "bytes" + "io/ioutil" + "net/http" + "net/http/httptest" + "testing" +) + +func TestHandlers(t *testing.T) { + testCases := []struct { + path string + handler http.HandlerFunc + statusCode int + contentType string + contentDisposition string + resp []byte + }{ + {"/debug/pprof/<script>scripty<script>", Index, http.StatusNotFound, "text/plain; charset=utf-8", "", []byte("Unknown profile\n")}, + {"/debug/pprof/heap", Index, http.StatusOK, "application/octet-stream", `attachment; filename="heap"`, nil}, + {"/debug/pprof/heap?debug=1", Index, http.StatusOK, "text/plain; charset=utf-8", "", nil}, + {"/debug/pprof/cmdline", Cmdline, http.StatusOK, "text/plain; charset=utf-8", "", nil}, + {"/debug/pprof/profile?seconds=1", Profile, http.StatusOK, "application/octet-stream", `attachment; filename="profile"`, nil}, + {"/debug/pprof/symbol", Symbol, http.StatusOK, "text/plain; charset=utf-8", "", nil}, + {"/debug/pprof/trace", Trace, http.StatusOK, "application/octet-stream", `attachment; filename="trace"`, nil}, + } + for _, tc := range testCases { + t.Run(tc.path, func(t *testing.T) { + req := httptest.NewRequest("GET", "http://example.com"+tc.path, nil) + w := httptest.NewRecorder() + tc.handler(w, req) + + resp := w.Result() + if got, want := resp.StatusCode, tc.statusCode; got != want { + t.Errorf("status code: got %d; want %d", got, want) + } + + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + t.Errorf("when reading response body, expected non-nil err; got %v", err) + } + if got, want := resp.Header.Get("X-Content-Type-Options"), "nosniff"; got != want { + t.Errorf("X-Content-Type-Options: got %q; want %q", got, want) + } + if got, want := resp.Header.Get("Content-Type"), tc.contentType; got != want { + t.Errorf("Content-Type: got %q; want %q", got, want) + } + if got, want := resp.Header.Get("Content-Disposition"), tc.contentDisposition; got != want { + t.Errorf("Content-Disposition: got %q; want %q", got, want) + } + + if resp.StatusCode == http.StatusOK { + return + } + if got, want := resp.Header.Get("X-Go-Pprof"), "1"; got != want { + t.Errorf("X-Go-Pprof: got %q; want %q", got, want) + } + if !bytes.Equal(body, tc.resp) { + t.Errorf("response: got %q; want %q", body, tc.resp) + } + }) + } + +} |