fix: reject requests with very long URIs

Some parts of the application may be vulnerable to very long URIs being passed.

E.g. Auth will try to save URI to session cookie, and it will fails, which will result in 500 error

Changelog: fixed

6 files changed, 123 insertions, 0 deletions

diff --git a/internal/config/config.go b/internal/config/config.go
index 3e03f7d6..c6f91db0 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -47,6 +47,7 @@ type Config struct {
 type General struct {
 	Domain          string
 	MaxConns        int
+	MaxURILength    int
 	MetricsAddress  string
 	RedirectHTTP    bool
 	RootCertificate []byte
@@ -181,6 +182,7 @@ func loadConfig() (*Config, error) {
 		General: General{
 			Domain:                     strings.ToLower(*pagesDomain),
 			MaxConns:                   *maxConns,
+			MaxURILength:               *maxURILength,
 			MetricsAddress:             *metricsAddress,
 			RedirectHTTP:               *redirectHTTP,
 			RootDir:                    *pagesRoot,
@@ -307,6 +309,8 @@ func LogConfig(config *Config) {
 		"enable-disk":                   config.GitLab.EnableDisk,
 		"auth-redirect-uri":             config.Authentication.RedirectURI,
 		"auth-scope":                    config.Authentication.Scope,
+		"max-conns":                     config.General.MaxConns,
+		"max-uri-length":                config.General.MaxURILength,
 		"zip-cache-expiration":          config.Zip.ExpirationInterval,
 		"zip-cache-cleanup":             config.Zip.CleanupInterval,
 		"zip-cache-refresh":             config.Zip.RefreshInterval,
diff --git a/internal/config/flags.go b/internal/config/flags.go
index c61447c7..6c9bd4a6 100644
--- a/internal/config/flags.go
+++ b/internal/config/flags.go
@@ -51,6 +51,7 @@ var (
 	redirectURI        = flag.String("auth-redirect-uri", "", "GitLab application redirect URI")
 	authScope          = flag.String("auth-scope", "api", "Scope to be used for authentication (must match GitLab Pages OAuth application settings)")
 	maxConns           = flag.Int("max-conns", 0, "Limit on the number of concurrent connections to the HTTP, HTTPS or proxy listeners, 0 for no limit")
+	maxURILength       = flag.Int("max-uri-length", 1024, "Limit the length of URI, 0 for unlimited.")
 	insecureCiphers    = flag.Bool("insecure-ciphers", false, "Use default list of cipher suites, may contain insecure ones like 3DES and RC4")
 	tlsMinVersion      = flag.String("tls-min-version", "tls1.2", tls.FlagUsage("min"))
 	tlsMaxVersion      = flag.String("tls-max-version", "", tls.FlagUsage("max"))
diff --git a/internal/httperrors/httperrors.go b/internal/httperrors/httperrors.go
index 8e61d590..a96e4d85 100644
--- a/internal/httperrors/httperrors.go
+++ b/internal/httperrors/httperrors.go
@@ -34,6 +34,14 @@ var (
      <p>Make sure the address is correct and that the page hasn't moved.</p>
      <p>Please contact your GitLab administrator if you think this is a mistake.</p>`,
 	}
+	content414 = content{
+		status:       http.StatusRequestURITooLong,
+		title:        "Request URI Too Long (414)",
+		statusString: "414",
+		header:       "Request URI Too Long.",
+		subHeader: `<p>The URI provided was too long for the server to process.</p>
+			<p>Try to make the request URI shorter.</p>`,
+	}
 
 	content429 = content{
 		http.StatusTooManyRequests,
@@ -184,6 +192,11 @@ func Serve404(w http.ResponseWriter) {
 	serveErrorPage(w, content404)
 }
 
+// Serve414 returns a 414 error response / HTML page to the http.ResponseWriter
+func Serve414(w http.ResponseWriter) {
+	serveErrorPage(w, content414)
+}
+
 // Serve429 returns a 429 error response / HTML page to the http.ResponseWriter
 func Serve429(w http.ResponseWriter) {
 	serveErrorPage(w, content429)
diff --git a/internal/httperrors/httperrors_test.go b/internal/httperrors/httperrors_test.go
index b003ce6f..671890f8 100644
--- a/internal/httperrors/httperrors_test.go
+++ b/internal/httperrors/httperrors_test.go
@@ -92,6 +92,18 @@ func TestServe404(t *testing.T) {
 	require.Contains(t, w.Content(), content404.subHeader)
 }
 
+func TestServe414(t *testing.T) {
+	w := newTestResponseWriter(httptest.NewRecorder())
+	Serve414(w)
+	require.Equal(t, w.Header().Get("Content-Type"), "text/html; charset=utf-8")
+	require.Equal(t, w.Header().Get("X-Content-Type-Options"), "nosniff")
+	require.Equal(t, w.Status(), content414.status)
+	require.Contains(t, w.Content(), content414.title)
+	require.Contains(t, w.Content(), content414.statusString)
+	require.Contains(t, w.Content(), content414.header)
+	require.Contains(t, w.Content(), content414.subHeader)
+}
+
 func TestServe500(t *testing.T) {
 	w := newTestResponseWriter(httptest.NewRecorder())
 	Serve500(w)
diff --git a/internal/urilimiter/urilimiter.go b/internal/urilimiter/urilimiter.go
new file mode 100644
index 00000000..65d664a5
--- /dev/null
+++ b/internal/urilimiter/urilimiter.go
@@ -0,0 +1,23 @@
+package urilimiter
+
+import (
+	"net/http"
+
+	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
+)
+
+func NewMiddleware(handler http.Handler, limit int) http.Handler {
+	if limit == 0 {
+		return handler
+	}
+
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if len(r.RequestURI) > limit {
+			httperrors.Serve414(w)
+
+			return
+		}
+
+		handler.ServeHTTP(w, r)
+	})
+}
diff --git a/internal/urilimiter/urilimiter_test.go b/internal/urilimiter/urilimiter_test.go
new file mode 100644
index 00000000..0ac89ea0
--- /dev/null
+++ b/internal/urilimiter/urilimiter_test.go
@@ -0,0 +1,70 @@
+package urilimiter
+
+import (
+	"fmt"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestNewMiddleware(t *testing.T) {
+	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprint(w, "hello")
+	})
+
+	tests := map[string]struct {
+		limit          int
+		url            string
+		expectedStatus int
+	}{
+		"with_disabled_middleware": {
+			limit:          0,
+			url:            "/index.html",
+			expectedStatus: http.StatusOK,
+		},
+		"with_limit_set_to_request_length": {
+			limit:          17,
+			url:            "/index.html?q=a#b",
+			expectedStatus: http.StatusOK,
+		},
+		"with_uri_length_exceeding_the_limit": {
+			limit:          17,
+			url:            "/index1.html?q=a#b",
+			expectedStatus: http.StatusRequestURITooLong,
+		},
+		"with_uri_length_exceeding_the_limit_with_query": {
+			limit:          17,
+			url:            "/index.html?q=aa#b",
+			expectedStatus: http.StatusRequestURITooLong,
+		},
+		"with_uri_length_exceeding_the_limit_with_fragment": {
+			limit:          17,
+			url:            "/index.html?q=a#bb",
+			expectedStatus: http.StatusRequestURITooLong,
+		},
+	}
+	for tn, tt := range tests {
+		t.Run(tn, func(t *testing.T) {
+			middleware := NewMiddleware(handler, tt.limit)
+
+			ww := httptest.NewRecorder()
+			rr := httptest.NewRequest(http.MethodGet, tt.url, nil)
+
+			middleware.ServeHTTP(ww, rr)
+
+			res := ww.Result()
+			defer res.Body.Close()
+
+			require.Equal(t, tt.expectedStatus, res.StatusCode)
+			if tt.expectedStatus == http.StatusOK {
+				b, err := io.ReadAll(res.Body)
+				require.NoError(t, err)
+
+				require.Equal(t, "hello", string(b))
+			}
+		})
+	}
+}