fix: Query predicate could be bypassed by prepared request, config is…

… enabled by default and you can disable with -validate-query=false or (#2028)

Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>

config/config.go

@@ -142,6 +142,7 @@ type Config struct {
 	NormalizeHost bool          `yaml:"normalize-host"`
 	HostPatch     net.HostPatch `yaml:"-"`
 
+	ValidateQuery bool      `yaml:"validate-query"`
 	RefusePayload multiFlag `yaml:"refuse-payload"`
 
 	// Kubernetes:
@@ -404,6 +405,7 @@ func NewConfig() *Config {
 	flag.Var(cfg.ForwardedHeadersExcludeCIDRList, "forwarded-headers-exclude-cidrs", "disables addition of forwarded headers for the remote host IPs from the comma separated list of CIDRs")
 
 	flag.BoolVar(&cfg.NormalizeHost, "normalize-host", false, "converts request host to lowercase and removes port and trailing dot if any")
+	flag.BoolVar(&cfg.ValidateQuery, "validate-query", true, "Validates the HTTP Query of a request and if invalid responds with status code 400")
 
 	flag.Var(&cfg.RefusePayload, "refuse-payload", "refuse requests that match configured value. Can be set multiple times")
 
@@ -920,6 +922,12 @@ func (c *Config) ToOptions() skipper.Options {
 		})
 	}
 
+	if c.ValidateQuery {
+		wrappers = append(wrappers, func(handler http.Handler) http.Handler {
+			return &net.ValidateQueryHandler{Handler: handler}
+		})
+	}
+
 	return options
 }
 

config/config_test.go

@@ -168,6 +168,7 @@ func Test_NewConfig(t *testing.T) {
 				ForwardedHeadersExcludeCIDRList:         commaListFlag(),
 				ClusterRatelimitMaxGroupShards:          1,
 				RefusePayload:                           multiFlag{"foo", "bar", "baz"},
+				ValidateQuery:                           true,
 			},
 			wantErr: false,
 		},

net/query.go

@@ -0,0 +1,18 @@
+package net
+
+import (
+	"net/http"
+	"net/url"
+)
+
+type ValidateQueryHandler struct {
+	Handler http.Handler
+}
+
+func (q *ValidateQueryHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	if _, err := url.ParseQuery(r.URL.RawQuery); err != nil {
+		http.Error(w, "Invalid query", http.StatusBadRequest)
+		return
+	}
+	q.Handler.ServeHTTP(w, r)
+}

net/query_test.go

@@ -0,0 +1,55 @@
+package net
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestQueryHandler(t *testing.T) {
+	for _, ti := range []struct {
+		name     string
+		routes   string
+		query    string
+		expected int
+	}{
+		{
+			name:     "request without query",
+			routes:   `r: * -> inlineContent("OK") -> <shunt>;`,
+			query:    "",
+			expected: http.StatusOK,
+		},
+		{
+			name:     "request with query",
+			routes:   `r1: Query("foo") -> status(400) -> inlineContent("FAIL") -> <shunt> ;r2: * -> inlineContent("OK") -> <shunt>;`,
+			query:    "foo=bar",
+			expected: http.StatusOK,
+		},
+		{
+			name:     "request with bad query",
+			routes:   `r1: Query("foo") -> status(400) -> inlineContent("FAIL") -> <shunt> ;r2: * -> inlineContent("OK") -> <shunt>;`,
+			query:    "foo=bar;",
+			expected: http.StatusBadRequest,
+		},
+	} {
+		t.Run(ti.name, func(t *testing.T) {
+
+			req, err := http.NewRequest("GET", "/path", nil)
+			if err != nil {
+				t.Fatal(err)
+			}
+			req.URL.RawQuery = ti.query
+
+			noop := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})
+			recorder := httptest.NewRecorder()
+			h := &ValidateQueryHandler{
+				Handler: noop,
+			}
+			h.ServeHTTP(recorder, req)
+
+			if recorder.Code != ti.expected {
+				t.Fatalf("Failed to get expected code %d, got %d", ti.expected, recorder.Code)
+			}
+		})
+	}
+}