From 98cd4333a1f71815cc050851361b77dc2ba312aa Mon Sep 17 00:00:00 2001
From: Matthew Holt <mholt@users.noreply.github.com>
Date: Tue, 14 Jun 2022 11:03:53 -0600
Subject: [PATCH] caddyhttp: Introduce strict HTTP mode

---
 modules/caddyhttp/app.go           | 10 ++++++++
 modules/caddyhttp/matchers.go      | 19 +++++++++------
 modules/caddyhttp/matchers_test.go | 13 ++++++++--
 modules/caddyhttp/server.go        | 38 ++++++++++++++++++++++++++++++
 4 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/modules/caddyhttp/app.go b/modules/caddyhttp/app.go
index d886b888..d3f39e87 100644
--- a/modules/caddyhttp/app.go
+++ b/modules/caddyhttp/app.go
@@ -111,6 +111,8 @@ type App struct {
 	// be forcefully closed.
 	GracePeriod caddy.Duration `json:"grace_period,omitempty"`
 
+	Strict *StrictOptions `json:"strict,omitempty"`
+
 	// Servers is the list of servers, keyed by arbitrary names chosen
 	// at your discretion for your own convenience; the keys do not
 	// affect functionality.
@@ -127,6 +129,13 @@ type App struct {
 	allCertDomains []string
 }
 
+type StrictOptions struct {
+	Disabled            bool `json:"disable,omitempty"`
+	LenientQueryStrings bool `json:"lenient_query_strings,omitempty"`
+	LenientPaths        bool `json:"lenient_paths,omitempty"`
+	LenientHeaders      bool `json:"lenient_headers,omitempty"`
+}
+
 // CaddyModule returns the Caddy module information.
 func (App) CaddyModule() caddy.ModuleInfo {
 	return caddy.ModuleInfo{
@@ -162,6 +171,7 @@ func (app *App) Provision(ctx caddy.Context) error {
 		srv.tlsApp = app.tlsApp
 		srv.logger = app.logger.Named("log")
 		srv.errorLogger = app.logger.Named("log.error")
+		srv.strict = app.Strict
 
 		// only enable access logs if configured
 		if srv.Logs != nil {
diff --git a/modules/caddyhttp/matchers.go b/modules/caddyhttp/matchers.go
index f8953ef5..471bc83f 100644
--- a/modules/caddyhttp/matchers.go
+++ b/modules/caddyhttp/matchers.go
@@ -503,15 +503,20 @@ func (m *MatchQuery) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 // Match returns true if r matches m. An empty m matches an empty query string.
 func (m MatchQuery) Match(r *http.Request) bool {
 	repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer)
+	reqQuery, err := url.ParseQuery(r.URL.RawQuery)
+	if err != nil {
+		return false
+	}
 	for param, vals := range m {
 		param = repl.ReplaceAll(param, "")
-		paramVal, found := r.URL.Query()[param]
-		if found {
-			for _, v := range vals {
-				v = repl.ReplaceAll(v, "")
-				if paramVal[0] == v || v == "*" {
-					return true
-				}
+		paramVal, found := reqQuery[param]
+		if !found {
+			continue
+		}
+		for _, v := range vals {
+			v = repl.ReplaceAll(v, "")
+			if paramVal[0] == v || v == "*" {
+				return true
 			}
 		}
 	}
diff --git a/modules/caddyhttp/matchers_test.go b/modules/caddyhttp/matchers_test.go
index f394921a..60331365 100644
--- a/modules/caddyhttp/matchers_test.go
+++ b/modules/caddyhttp/matchers_test.go
@@ -624,9 +624,18 @@ func TestQueryMatcher(t *testing.T) {
 			input:    "/?somekey=1",
 			expect:   true,
 		},
+		{
+			scenario: "invalid query string",
+			match:    MatchQuery{"test": []string{"*"}},
+			input:    "/?test=1;",
+			expect:   false,
+		},
 	} {
-
-		u, _ := url.Parse(tc.input)
+		u, err := url.Parse(tc.input)
+		if err != nil {
+			t.Errorf("Test %d: Parsing URL: %v", i, err)
+			continue
+		}
 
 		req := &http.Request{URL: u}
 		repl := caddy.NewReplacer()
diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go
index a4a976f7..8a784318 100644
--- a/modules/caddyhttp/server.go
+++ b/modules/caddyhttp/server.go
@@ -132,6 +132,7 @@ type Server struct {
 	primaryHandlerChain Handler
 	errorHandlerChain   Handler
 	listenerWrappers    []caddy.ListenerWrapper
+	strict              *StrictOptions
 
 	tlsApp       *caddytls.TLS
 	logger       *zap.Logger
@@ -315,9 +316,46 @@ func (s *Server) enforcementHandler(w http.ResponseWriter, r *http.Request, next
 			return Error(http.StatusMisdirectedRequest, err)
 		}
 	}
+
+	if err := s.strict.enforce(r); err != nil {
+		return err
+	}
+
 	return next.ServeHTTP(w, r)
 }
 
+func (strict *StrictOptions) enforce(r *http.Request) error {
+	if strict != nil && strict.Disabled {
+		return nil
+	}
+
+	// Reject query strings with unencoded ;
+	if strict == nil || !strict.LenientQueryStrings {
+		_, err := url.ParseQuery(r.URL.RawQuery)
+		if err != nil {
+			return Error(http.StatusBadRequest, fmt.Errorf("invalid query string: %w", err))
+		}
+	}
+
+	// Reject header fields with _ in name (#4830)
+	if strict == nil || !strict.LenientHeaders {
+		for field := range r.Header {
+			if strings.Contains(field, "_") {
+				return Error(http.StatusBadRequest, fmt.Errorf("invalid header field name: %s", field))
+			}
+		}
+	}
+
+	// Reject paths with // or ..
+	if strict == nil || !strict.LenientPaths {
+		if strings.Contains(r.URL.Path, "//") || strings.Contains(r.URL.Path, "..") {
+			return Error(http.StatusBadRequest, fmt.Errorf("invalid request path: %s", r.URL.RawPath))
+		}
+	}
+
+	return nil
+}
+
 // listenersUseAnyPortOtherThan returns true if there are any
 // listeners in s that use a port which is not otherPort.
 func (s *Server) listenersUseAnyPortOtherThan(otherPort int) bool {