net/http: readRequest fails to handle URIs ending with space characters

[colin969]

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

$ go version
go version go1.20.2 windows/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
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\colin\AppData\Local\go-build
set GOENV=C:\Users\colin\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\colin\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\colin\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.20.2
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=J:\Data\Projects\fpProxy\go.mod
set GOWORK=J:\Data\Projects\fpProxy\go.work
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=C:\Users\colin\AppData\Local\Temp\go-build4117129570=/tmp/go-build -gno-record-gcc-switches

What did you do?

I work with particularly old software, which in some cases will send HTTP requests without stripping whitespace from the Request URI. After changing our proxy to be Go based, these requests just weren't being received by the proxy between the application and the file server.

When putting a Python or Fiddler proxy between the application and Go proxy server the requests would suddenly start working. The only idea I could come up with is that they're sanitizing the requests are they pass through.

I went digging and found that when reading the request, parseRequestLine works on the assumption that each element of the request line is seperated by a single space character, as defined in the RFC. When it splits, the HTTP proto gets an extra space before it. This means it'll error as a malformed proto. I know this behaviour isn't strictly supported by the RFC but I'd like to see it reflect how other software handles it.

The easiest way to test this is to send variants of GET /test HTTP/1.0 and GET /test HTTP/1.0 with something like telnet to a Go HTTP server, where the latter will return 400 Bad Request.

For demonstration just to display where the space prefixing the proto comes from, I took a copy of the current parseRequestLine and then fed the proto into ParseHTTPVersion

https://go.dev/play/p/8F2Q9A5Fhx1

What did you expect to see?

'GET', 'http://www.nabiscoworld.com/shock/nbgrm.dcr', 'HTTP/1.0'

What did you see instead?

'GET', 'http://www.nabiscoworld.com/shock/nbgrm.dcr', ' HTTP/1.0'
Malformed proto

[seankhliao]

cc @neild

[krum110487]

Ideally, it would be nice to optionally have the functionality like this, to allow them to pass through, but escaped.

https://go.dev/play/p/fkBi238Kgav

[bcmills]

I know this behaviour isn't strictly supported by the RFC but I'd like to see it reflect how other software handles it.

Generally Go's net/http tries to stick to the RFCs unless the deviations are used widely. Can you give some examples of widely-used clients that require this behavior, and perhaps more detail about why it cannot be fixed in those clients instead?

[colin969]

It's a tricky one - It isn't commonly used and virtually any client in the past 15+ years shouldn't ever do this, but we're finding incorrect use of it in some Adobe Shockwave games, games closer to 20-25 years old.

The behavior is reflected in every other software I've tried - Fiddler, Python, Apache etc all handle it fine (in fact the http://go.dev/ site itself considers it valid). I've really struggled to find documentation for why they support it in the first place, the only thing I could dig up was this section of the URI Generic Syntax RFC from 1998 under 'Syntax Notation and Common Elements', but it was removed by the time the current RFC from 2005 appears.

<first>/<second>;<third>?<fourth>

The component names are enclosed in angle-brackets and any characters
outside angle-brackets are literal separators. Whitespace should be
ignored. These descriptions are used informally and do not define
the syntax requirements.

Obviously having this supported is something I'd like considering our use case, but I'm understanding of why you might also choose not to.

[krum110487]

Just to add some more samples of it working Using OpenSSL to test, go.dev worked with any number of spaces

All valid (Including spaces AFTER HTTP/1.1)
GET go.dev  HTTP/1.1
GET go.dev    HTTP/1.1
GET        go.dev      HTTP/1.1
GET   go.dev/play/       HTTP/1.1   
GET   https://www.google.com/imghp?hl=en&ogbl       HTTP/1.1
GET        https://learn.microsoft.com/en-us/    HTTP/1.1   
GET        https://httpd.apache.org/    HTTP/1.1   

Invalid (Returns 400)
  GET go.dev HTTP/1.1
GET go.dev/play / HTTP/1.1
GET go.dev/pl ay/ HTTP/1.1

I tried to find other examples of nginx and apache, but I couldn't get any request to work openssl with nginx.org or facebook.com, everything was rejected, I am sure I am doing something wrong.

All sites rejected anything starting with a space, but every site I tried that I could get to work normally worked with multiple spaces between Method and URI and URI and Protocol and Protocol to end.

[jskcnsl]

We use Golang to parse raw http request in cybersecurity, and there is an easy sql inject example, which can't be parsed by http.ReadRequest.

$ curl -vv "http://10.1.1.2:8010/?id=1 or 1=2"
*   Trying 10.1.1.2:8010...
* TCP_NODELAY set
* Connected to 10.1.1.2 (10.1.1.2) port 8010 (#0)
> GET /?id=1 or 1=2 HTTP/1.1
> Host: 10.1.1.2:8010
> User-Agent: curl/7.68.0
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Server: Test-Server
< Date: Tue, 02 Jan 2024 10:16:52 GMT
< Content-Length: 0
<

Of cause, this is not a Generally http request, but we want to use Go do more 😁

[seankhliao]

cc @golang/security

sounds like we should reject making the parser any laxer

[jskcnsl]

cc @golang/security

sounds like we should reject making the parser any laxer

I mean we need more laxer to make Golang can be used to parse these no generally request. 😂
We want to get a key-value pair from http query "id" = "1 or 1=2".