[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>

diff --git a/src/net/http/pprof/pprof.go b/src/net/http/pprof/pprof.go
index 12c7599ab0fac8b598e2b6823ee331f1e0082fcc..7f01ee47a045aaf7f0100ba2d2ba056e8e0affd4 100644 (file)
--- 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 (file)
index 0000000..47dd35b
--- /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)
+                       }
+               })
+       }
+
+}