Skip to content

Commit 1b3eee4

Browse files
Reject backslash absolute URIs and cache parse errors
Keep our server behaviour the same as net/http.
1 parent 18bceed commit 1b3eee4

File tree

4 files changed

+61
-9
lines changed

4 files changed

+61
-9
lines changed

http.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ type Request struct {
6666
// Group bool members in order to reduce Request object size.
6767
parsedURI bool
6868
parsedPostArgs bool
69+
uriParseErr error
6970

7071
keepBodyBuffer bool
7172

@@ -146,12 +147,14 @@ func (req *Request) Host() []byte {
146147
func (req *Request) SetRequestURI(requestURI string) {
147148
req.Header.SetRequestURI(requestURI)
148149
req.parsedURI = false
150+
req.uriParseErr = nil
149151
}
150152

151153
// SetRequestURIBytes sets RequestURI.
152154
func (req *Request) SetRequestURIBytes(requestURI []byte) {
153155
req.Header.SetRequestURIBytes(requestURI)
154156
req.parsedURI = false
157+
req.uriParseErr = nil
155158
}
156159

157160
// RequestURI returns request's URI.
@@ -891,6 +894,7 @@ func (req *Request) copyToSkipBody(dst *Request) {
891894

892895
req.uri.CopyTo(&dst.uri)
893896
dst.parsedURI = req.parsedURI
897+
dst.uriParseErr = req.uriParseErr
894898

895899
req.postArgs.CopyTo(&dst.postArgs)
896900
dst.parsedPostArgs = req.parsedPostArgs
@@ -960,19 +964,22 @@ func (req *Request) SetURI(newURI *URI) {
960964
if newURI != nil {
961965
newURI.CopyTo(&req.uri)
962966
req.parsedURI = true
967+
req.uriParseErr = nil
963968
return
964969
}
965970
req.uri.Reset()
966971
req.parsedURI = false
972+
req.uriParseErr = nil
967973
}
968974

969975
func (req *Request) parseURI() error {
970976
if req.parsedURI {
971-
return nil
977+
return req.uriParseErr
972978
}
973-
req.parsedURI = true
974979

975-
return req.uri.parse(req.Header.Host(), req.Header.RequestURI(), req.isTLS)
980+
req.parsedURI = true
981+
req.uriParseErr = req.uri.parse(req.Header.Host(), req.Header.RequestURI(), req.isTLS)
982+
return req.uriParseErr
976983
}
977984

978985
// PostArgs returns POST arguments.
@@ -1146,6 +1153,7 @@ func (req *Request) resetSkipHeader() {
11461153
req.ResetBody()
11471154
req.uri.Reset()
11481155
req.parsedURI = false
1156+
req.uriParseErr = nil
11491157
req.postArgs.Reset()
11501158
req.parsedPostArgs = false
11511159
req.isTLS = false

http_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,8 @@ func testRequestCopyTo(t *testing.T, src *Request) {
155155
var dst Request
156156
src.CopyTo(&dst)
157157

158-
if !reflect.DeepEqual(src, &dst) {
158+
// Compare serialized representations.
159+
if src.String() != dst.String() || !bytes.Equal(src.Body(), dst.Body()) {
159160
t.Fatalf("RequestCopyTo fail, src: \n%+v\ndst: \n%+v\n", src, &dst)
160161
}
161162
}

server.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2349,11 +2349,21 @@ func (s *Server) serveConn(c net.Conn) error {
23492349
writeTimeout = s.WriteTimeout
23502350
}
23512351
}
2352-
// read body
2353-
if s.StreamRequestBody {
2354-
err = ctx.Request.readBodyStream(br, maxRequestBodySize, s.GetOnly, !s.DisablePreParseMultipartForm)
2355-
} else {
2356-
err = ctx.Request.readLimitBody(br, maxRequestBodySize, s.GetOnly, !s.DisablePreParseMultipartForm)
2352+
2353+
if err == nil {
2354+
if err = ctx.Request.parseURI(); err != nil {
2355+
bw = s.writeErrorResponse(bw, ctx, serverName, err)
2356+
break
2357+
}
2358+
}
2359+
2360+
if err == nil {
2361+
// read body
2362+
if s.StreamRequestBody {
2363+
err = ctx.Request.readBodyStream(br, maxRequestBodySize, s.GetOnly, !s.DisablePreParseMultipartForm)
2364+
} else {
2365+
err = ctx.Request.readLimitBody(br, maxRequestBodySize, s.GetOnly, !s.DisablePreParseMultipartForm)
2366+
}
23572367
}
23582368
}
23592369
// When StreamRequestBody is set to true, we cannot safely release br.

server_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"runtime"
1717
"strings"
1818
"sync"
19+
"sync/atomic"
1920
"testing"
2021
"time"
2122

@@ -1882,6 +1883,38 @@ func TestServerHeadRequest(t *testing.T) {
18821883
}
18831884
}
18841885

1886+
func TestServerRejectsBackslashInAbsoluteURI(t *testing.T) {
1887+
t.Parallel()
1888+
1889+
var handlerCalled atomic.Bool
1890+
s := &Server{
1891+
Handler: func(ctx *RequestCtx) {
1892+
handlerCalled.Store(true)
1893+
ctx.Success("text/plain", []byte("ok"))
1894+
},
1895+
}
1896+
1897+
rw := &readWriter{}
1898+
rw.r.WriteString("GET http://vulndetector.com\\\\admin\\\\api HTTP/1.1\r\nHost: example.com\r\n\r\n")
1899+
1900+
_ = s.ServeConn(rw)
1901+
1902+
br := bufio.NewReader(&rw.w)
1903+
verifyResponse(t, br, StatusBadRequest, string(defaultContentType), "Error when parsing request")
1904+
1905+
if handlerCalled.Load() {
1906+
t.Fatal("handler should not run for invalid absolute URI")
1907+
}
1908+
1909+
data, err := io.ReadAll(br)
1910+
if err != nil {
1911+
t.Fatalf("Unexpected error when reading remaining data: %v", err)
1912+
}
1913+
if len(data) > 0 {
1914+
t.Fatalf("unexpected remaining data %q", data)
1915+
}
1916+
}
1917+
18851918
func TestServerExpect100Continue(t *testing.T) {
18861919
t.Parallel()
18871920

0 commit comments

Comments
 (0)