Patched Fix Inconsistent Interpretation of HTTP Requests Smuggling

[imhunterand]

A request smuggling attack is possible when using MaxBytesHandler. When using MaxBytesHandler, the body of an HTTP request is not fully consumed. When the server attempts to read HTTP2 frames from the connection, it will instead be reading the body of the HTTP request, which could be attacker-manipulated to represent arbitrary HTTP2 requests.

func main() {
    h2s := &http2.Server{}

    handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        PrintMemUsage()
        n, err := io.Copy(io.Discard, r.Body)
        fmt.Fprintf(w, "http: %v, res: %v/%v\n", r.Proto, n, err)
        log.Printf("http: %v, res: %v/%v\n", r.Proto, n, err)
        PrintMemUsage()
    })

    server := &http.Server{
        Addr:    "0.0.0.0:8888",
        Handler: h2c.NewHandler(handler, h2s),
    }

    fmt.Printf("Listening [0.0.0.0:8888]...\n")
    log.Println(server.ListenAndServe())
}
func PrintMemUsage() {
    var m runtime.MemStats
    runtime.ReadMemStats(&m)
    // For info on each, see: https://golang.org/pkg/runtime/#MemStats
    fmt.Printf("Alloc = %v MiB", bToMb(m.Alloc))
    fmt.Printf("\tTotalAlloc = %v MiB", bToMb(m.TotalAlloc))
    fmt.Printf("\tSys = %v MiB", bToMb(m.Sys))
    fmt.Printf("\tNumGC = %v\n", m.NumGC)
}

func bToMb(b uint64) uint64 {
    return b / 1024 / 1024
}

curl -X POST -d @output.dat localhost:8888
curl -X POST -d @output.dat localhost:8888 --http2-prior-knowledge
curl -X POST -d @output.dat localhost:8888 --http2

func MaxBytesHandlerForH2CUpdate(h http.Handler, n int64) http.Handler {
    mbh := http.MaxBytesHandler(h, n)
    return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        if isH2CUpgrade(r.Header) {
            mbh.ServeHTTP(w, r)
        }
        h.ServeHTTP(w, r)
    })
}

func isH2CUpgrade(h http.Header) bool {
    return httpguts.HeaderValuesContainsToken(h[textproto.CanonicalMIMEHeaderKey("Upgrade")], "h2c") &&
        httpguts.HeaderValuesContainsToken(h[textproto.CanonicalMIMEHeaderKey("Connection")], "HTTP2-Settings")
}

Impact

CWE-444 CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H

• Targeted PR against correct branch (see CONTRIBUTING.md)

• [ ] Linked to github-issue with discussion and accepted design OR link to spec that describes this work.

• [x] Wrote tests

• [x] Updated relevant documentation (docs/)

• [x] Added a relevant changelog entry to the Unreleased section in CHANGELOG.md

• [x] Reviewed Files changed in the github PR explorer

---

For Admin Use:

• Added appropriate labels to PR (ex. wip, ready-for-review, docs)
• Reviewers Assigned
• Squashed all commits, uses message "Merge PR #XYZ: [title]" (coding standards)