Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

net/http: server permits handlers to write headers with newlines #47711

Closed
Az3z3l opened this issue Aug 15, 2021 · 5 comments
Closed

net/http: server permits handlers to write headers with newlines #47711

Az3z3l opened this issue Aug 15, 2021 · 5 comments
Labels
FrozenDueToAge Security
Milestone
Backlog

Comments

@Az3z3l
Copy link

Az3z3l commented Aug 15, 2021

What version of Go are you using (go version)?

$ go version
go version go1.17rc1 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17rc1"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/go/src/crlfpoc/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2202782937=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.17rc1 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.17rc1
uname -sr: Linux 5.10.52-1-MANJARO
/lib/x86_64-linux-gnu/libc.so.6: GNU C Library (Debian GLIBC 2.28-10) stable release version 2.28.

What did you do?

Consider the following code:

package main

import (
	"fmt"
	"log"
	"net/http"
)

func main() {
	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
		headerName := r.URL.Query().Get("name")
		headerValue := r.URL.Query().Get("value")
		w.Header().Set(headerName, headerValue)
		fmt.Fprintf(w, "Hello World")
	})

	fmt.Printf("Starting server at port 3000\n")
	if err := http.ListenAndServe(":3000", nil); err != nil {
		log.Fatal(err)
	}
}

When requesting http://localhost:3000/?name=%0a%0a%3Chtml%3E%3Cscript%3Ealert(%27not%20supposed%20to%20happen%27)%3C/script%3Easd&value=a%0aasd, the name parameter's value (name=%0a%0a<html><script>alert('not supposed to happen')</script>) is set as the header's name. But due to no sanitization, %0a can be used to inject new headers or arbitrary HTML content.

What did you expect to see?

%0a in the header's value is sanitized and causes no CRLF. The same could be expected in the case of header's name.

What did you see instead?

Proper sanitization of the header's name.
@bradfitz
Copy link
Contributor

Thanks. A better repro, as the query param part is fine and a bit of a distraction:

https://play.golang.org/p/v7ghPz4CfxT

func main() {
	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Foo", "Bar")
		w.Header().Set("Xxx: xxxx\nSmuggle", "smuggleval")
	}))
	res, err := http.Get(ts.URL)
	if err != nil {
		log.Fatal(err)
	}
	res.Write(os.Stdout)
	fmt.Printf("%v", res.Header)
}

Produces:

HTTP/1.1 200 OK
Date: Tue, 10 Nov 2009 23:00:00 GMT
Foo: Bar
Smuggle: smuggleval
Xxx: xxxx
Content-Length: 0

map[Content-Length:[0] Date:[Tue, 10 Nov 2009 23:00:00 GMT] Foo:[Bar] Smuggle:[smuggleval] Xxx:[xxxx]]

@bradfitz bradfitz changed the title CRLF in Go's Header().Set() function net/http: Server permits handlers to write headers with newlines Aug 16, 2021
@Captain-K-101 Captain-K-101 mentioned this issue Aug 16, 2021
Closed
@mknyszek mknyszek changed the title net/http: Server permits handlers to write headers with newlines net/http: server permits handlers to write headers with newlines Aug 16, 2021
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 16, 2021
@mknyszek mknyszek added this to the Backlog milestone Aug 16, 2021
@mknyszek
Copy link
Contributor

@Az3z3l @bradfitz Is this new in Go 1.17?

CC @neild via https://dev.golang.org/owners

@neild
Copy link
Contributor

neild commented Aug 16, 2021

@Az3z3l @bradfitz Is this new in Go 1.17?

Also present in 1.16.

@neild neild self-assigned this Aug 16, 2021
@gopherbot
Copy link

Change https://golang.org/cl/342530 mentions this issue: net/http: drop headers with invalid keys in Header.Write

@neild neild removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 16, 2021
@rsc rsc unassigned neild Jun 23, 2022
@catenacyber
Copy link
Contributor

Thanks for this fix ;-)

@golang golang locked and limited conversation to collaborators Aug 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Security
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
6 participants
@bradfitz @neild @mknyszek @gopherbot @catenacyber @Az3z3l