BED-5301/5298: PenTest Result: Internal IP Address Disclosure AND Rate limit Bypass

cmd/api/src/api/middleware/rate_limit.go

@@ -17,66 +17,42 @@
 package middleware
 
 import (
+	"log/slog"
 	"net/http"
+	"strings"
 	"time"
 
-	"github.com/specterops/bloodhound/headers"
-
-	"github.com/didip/tollbooth/v6"
-	"github.com/didip/tollbooth/v6/limiter"
 	"github.com/gorilla/mux"
-	"github.com/specterops/bloodhound/src/api"
+	"github.com/ulule/limiter/v3"
+	"github.com/ulule/limiter/v3/drivers/middleware/stdlib"
+	"github.com/ulule/limiter/v3/drivers/store/memory"
 )
 
 // DefaultRateLimit is the default number of allowed requests per second
 const DefaultRateLimit = 55
 
-// RateLimitHandler returns a http.Handler that limits the rate of requests
-// for a given handler
-//
-// Usage:
-//
-//	limiter := tollbooth.NewLimiter(1, nil).
-//		SetMethods([]string{"GET", "POST"}).
-//		...configure tollbooth Limiter ...
-//
-//	router.Handle("/teapot", RateLimitHandler(limiter, handler))
-func RateLimitHandler(limiter *limiter.Limiter, handler http.Handler) http.Handler {
-	return http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
-		if err := tollbooth.LimitByRequest(limiter, response, request); err != nil {
-			// In case SetOnLimitReached was called
-			limiter.ExecOnLimitReached(response, request)
-
-			api.WriteErrorResponse(request.Context(), &api.ErrorWrapper{
-				HTTPStatus: err.StatusCode,
-				Timestamp:  time.Now(),
-				RequestID:  request.Header.Get(headers.RequestID.String()),
-				Errors: []api.ErrorDetails{
-					{
-						Context: "middleware",
-						Message: err.Error(),
-					},
-				},
-			}, response)
-		} else {
-			handler.ServeHTTP(response, request)
-		}
-	})
-}
-
 // RateLimitMiddleware is a convenience function for creating rate limiting middleware
 //
-// Usage:
-//
-//	limiter := tollbooth.NewLimiter(1, nil).
-//		SetMethods([]string{"GET", "POST"}).
-//		...configure tollbooth Limiter ...
-//
 //	router.Use(RateLimitMiddleware(limiter))
 func RateLimitMiddleware(limiter *limiter.Limiter) mux.MiddlewareFunc {
-	return func(next http.Handler) http.Handler {
-		return RateLimitHandler(limiter, next)
-	}
+	ipGetter := stdlib.WithKeyGetter(
+		func(r *http.Request) string {
+			if xff := r.Header.Get("X-Forwarded-For"); xff == "" {
+				slog.DebugContext(r.Context(), "No data found in X-Forwarded-For header")
+				return r.RemoteAddr
+			} else {
+				ips := strings.Split(xff, ",")
+				rightMostIP := strings.TrimSpace(ips[len(ips)-1])
+
+				return rightMostIP
+			}
+		},
+	)
+
+	middleware := stdlib.NewMiddleware(limiter, ipGetter)
+
+	return middleware.Handler
+
 }
 
 // DefaultRateLimitMiddleware is a convenience function for creating the default rate limiting middleware
@@ -86,6 +62,14 @@ func RateLimitMiddleware(limiter *limiter.Limiter) mux.MiddlewareFunc {
 //
 //	router.Use(DefaultRateLimitMiddleware())
 func DefaultRateLimitMiddleware() mux.MiddlewareFunc {
-	limiter := tollbooth.NewLimiter(DefaultRateLimit, nil)
-	return RateLimitMiddleware(limiter)
+	rate := limiter.Rate{
+		Period: 1 * time.Second,
+		Limit:  DefaultRateLimit,
+	}
+
+	store := memory.NewStore()
+
+	instance := limiter.New(store, rate, limiter.WithTrustForwardHeader(false))
+
+	return RateLimitMiddleware(instance)
 }

cmd/api/src/api/middleware/rate_limit_test.go

@@ -20,72 +20,28 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
+	"time"
 
-	"github.com/didip/tollbooth/v6"
 	"github.com/gorilla/mux"
 	"github.com/specterops/bloodhound/src/api/middleware"
+	"github.com/ulule/limiter/v3"
+	"github.com/ulule/limiter/v3/drivers/store/memory"
 )
 
-func TestRateLimitHandler(t *testing.T) {
-
-	// Limit to 1 req/s
-	limiter := tollbooth.NewLimiter(1, nil)
-
-	count_429 := 0
-	limiter.SetOnLimitReached(func(response http.ResponseWriter, request *http.Request) {
-		count_429++
-	})
-
-	if req, err := http.NewRequest("GET", "/teapot", nil); err != nil {
-		t.Fatal(err)
-	} else {
-		req.Header.Set("X-Real-IP", "8.8.8.8")
-
-		handler := middleware.RateLimitHandler(limiter, &CountingHandler{})
-		router := mux.NewRouter()
-		router.Handle("/teapot", handler)
-
-		rr := httptest.NewRecorder()
-		router.ServeHTTP(rr, req)
-
-		// Expect a 200
-		if status := rr.Code; status != http.StatusOK {
-			t.Errorf("handler returned wrong status code: got %v want %v", status, http.StatusOK)
-		}
-
-		ch := make(chan int)
-		go func() {
-			rr := httptest.NewRecorder()
-			router.ServeHTTP(rr, req)
-
-			// Expect a 429
-			if status := rr.Code; status != http.StatusTooManyRequests {
-				t.Errorf("handler returned wrong status code: got %v want %v", status, http.StatusTooManyRequests)
-			}
-
-			if count_429 == 0 {
-				t.Error("OnLimitReached callback function should have been called")
-			}
-
-			close(ch)
-		}()
-		<-ch
-	}
-}
-
 func TestRateLimitMiddleware(t *testing.T) {
-
 	allowedReqsPerSecond := 5
-	limiter := tollbooth.NewLimiter(float64(allowedReqsPerSecond), nil)
+	rate := limiter.Rate{
+		Period: 1 * time.Second,
+		Limit:  int64(allowedReqsPerSecond),
+	}
 
-	count_429 := 0
-	limiter.SetOnLimitReached(func(w http.ResponseWriter, r *http.Request) {
-		count_429++
-	})
+	store := memory.NewStore()
+
+	instance := limiter.New(store, rate, limiter.WithTrustForwardHeader(false))
 
 	testHandler := &CountingHandler{}
 	router := mux.NewRouter()
-	router.Use(middleware.RateLimitMiddleware(limiter))
+	router.Use(middleware.RateLimitMiddleware(instance))
 	router.Handle("/teapot", testHandler)
 
 	if req, err := http.NewRequest("GET", "/teapot", nil); err != nil {
@@ -103,10 +59,6 @@ func TestRateLimitMiddleware(t *testing.T) {
 	if testHandler.Count != allowedReqsPerSecond {
 		t.Errorf("invalid HTTP 200 count: got %v want %v", testHandler.Count, 5)
 	}
-
-	if count_429 != 1 {
-		t.Errorf("invalid HTTP 429 count: got %v want %v", count_429, 1)
-	}
 }
 
 func TestDefaultRateLimitMiddleware(t *testing.T) {

cmd/api/src/api/registration/v2.go

@@ -19,7 +19,9 @@ package registration
 import (
 	"fmt"
 	"net/http"
+	"time"
 
+	"github.com/gorilla/mux"
 	"github.com/specterops/bloodhound/openapi"
 	"github.com/specterops/bloodhound/params"
 	"github.com/specterops/bloodhound/src/api"
@@ -29,6 +31,8 @@ import (
 	authapi "github.com/specterops/bloodhound/src/api/v2/auth"
 	"github.com/specterops/bloodhound/src/auth"
 	"github.com/specterops/bloodhound/src/model/appcfg"
+	"github.com/ulule/limiter/v3"
+	"github.com/ulule/limiter/v3/drivers/store/memory"
 )
 
 func registerV2Auth(resources v2.Resources, routerInst *router.Router, permissions auth.PermissionSet) {
@@ -37,9 +41,23 @@ func registerV2Auth(resources v2.Resources, routerInst *router.Router, permissio
 		managementResource = authapi.NewManagementResource(resources.Config, resources.DB, resources.Authorizer, resources.Authenticator)
 	)
 
+	router.With(func() mux.MiddlewareFunc {
+		rate := limiter.Rate{
+			Period: 1 * time.Second,
+			Limit:  1,
+		}
+
+		store := memory.NewStore()
+
+		instance := limiter.New(store, rate, limiter.WithTrustForwardHeader(false))
+		return middleware.RateLimitMiddleware(instance)
+	},
+		// Login resource
+		routerInst.POST("/api/v2/login", loginResource.Login),
+	)
+
 	router.With(middleware.DefaultRateLimitMiddleware,
 		// Login resources
-		routerInst.POST("/api/v2/login", loginResource.Login),
 		routerInst.GET("/api/v2/self", managementResource.GetSelf),
 		routerInst.POST("/api/v2/logout", loginResource.Logout),
 

cmd/api/src/go.mod

@@ -23,7 +23,6 @@ require (
 	github.com/channelmeter/iso8601duration v0.0.0-20150204201828-8da3af7a2a61
 	github.com/coreos/go-oidc/v3 v3.11.0
 	github.com/crewjam/saml v0.4.14
-	github.com/didip/tollbooth/v6 v6.1.2
 	github.com/go-chi/chi/v5 v5.0.8
 	github.com/gobeam/stringy v0.0.6
 	github.com/gofrs/uuid v4.4.0+incompatible
@@ -41,6 +40,7 @@ require (
 	github.com/russellhaering/goxmldsig v1.4.0
 	github.com/stretchr/testify v1.9.0
 	github.com/teambition/rrule-go v1.8.2
+	github.com/ulule/limiter/v3 v3.11.2
 	github.com/unrolled/secure v1.13.0
 	go.uber.org/mock v0.2.0
 	golang.org/x/oauth2 v0.23.0
@@ -57,7 +57,6 @@ require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/felixge/httpsnoop v1.0.3 // indirect
 	github.com/go-jose/go-jose/v4 v4.0.2 // indirect
-	github.com/go-pkgz/expirable-cache v1.0.0 // indirect
 	github.com/golang/protobuf v1.5.3 // indirect
 	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/jackc/pgpassfile v1.0.0 // indirect
@@ -79,7 +78,6 @@ require (
 	golang.org/x/sync v0.10.0 // indirect
 	golang.org/x/sys v0.28.0 // indirect
 	golang.org/x/text v0.21.0 // indirect
-	golang.org/x/time v0.3.0 // indirect
 	google.golang.org/protobuf v1.34.1 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )

cmd/api/src/go.sum

@@ -22,18 +22,13 @@ github.com/crewjam/saml v0.4.14/go.mod h1:UVSZCf18jJkk6GpWNVqcyQJMD5HsRugBPf4I1n
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
-github.com/didip/tollbooth/v6 v6.1.2 h1:Kdqxmqw9YTv0uKajBUiWQg+GURL/k4vy9gmLCL01PjQ=
-github.com/didip/tollbooth/v6 v6.1.2/go.mod h1:xjcse6CTHCLuOkzsWrEgdy9WPJFv+p/x6v+MyfP+O9s=
 github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
 github.com/felixge/httpsnoop v1.0.3 h1:s/nj+GCswXYzN5v2DpNMuMQYe+0DDwt5WVCU6CWBdXk=
 github.com/felixge/httpsnoop v1.0.3/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
 github.com/go-chi/chi/v5 v5.0.8 h1:lD+NLqFcAi1ovnVZpsnObHGW4xb4J8lNmoYVfECH1Y0=
 github.com/go-chi/chi/v5 v5.0.8/go.mod h1:DslCQbL2OYiznFReuXYUmQ2hGd1aDpCnlMNITLSKoi8=
 github.com/go-jose/go-jose/v4 v4.0.2 h1:R3l3kkBds16bO7ZFAEEcofK0MkrAJt3jlJznWZG0nvk=
 github.com/go-jose/go-jose/v4 v4.0.2/go.mod h1:WVf9LFMHh/QVrmqrOfqun0C45tMe3RoiKJMPvgWwLfY=
-github.com/go-pkgz/expirable-cache v0.0.3/go.mod h1:+IauqN00R2FqNRLCLA+X5YljQJrwB179PfiAoMPlTlQ=
-github.com/go-pkgz/expirable-cache v1.0.0 h1:ns5+1hjY8hntGv8bPaQd9Gr7Jyo+Uw5SLyII40aQdtA=
-github.com/go-pkgz/expirable-cache v1.0.0/go.mod h1:GTrEl0X+q0mPNqN6dtcQXksACnzCBQ5k/k1SwXJsZKs=
 github.com/gobeam/stringy v0.0.6 h1:IboItevQArUAYUbjb7xmtGoJfN5Aqpk3/bVCd7JgWe0=
 github.com/gobeam/stringy v0.0.6/go.mod h1:W3620X9dJHf2FSZF5fRnWekHcHQjwmCz8ZQ2d1qloqE=
 github.com/gofrs/uuid v4.4.0+incompatible h1:3qXRTX8/NbyulANqlc0lchS1gqAVxRgsuW1YrTJupqA=
@@ -113,13 +108,13 @@ github.com/rivo/uniseg v0.4.4/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUc
 github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc=
 github.com/rogpeppe/go-internal v1.8.0/go.mod h1:WmiCO8CzOY8rg0OYDC4/i/2WRWAB6poM+XZ2dLUbcbE=
 github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII=
+github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o=
 github.com/russellhaering/goxmldsig v1.4.0 h1:8UcDh/xGyQiyrW+Fq5t8f+l2DLB1+zlhYzkPUJ7Qhys=
 github.com/russellhaering/goxmldsig v1.4.0/go.mod h1:gM4MDENBQf7M+V824SGfyIUVFWydB7n0KkEubVJl+Tw=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
-github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
@@ -128,6 +123,8 @@ github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsT
 github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
 github.com/teambition/rrule-go v1.8.2 h1:lIjpjvWTj9fFUZCmuoVDrKVOtdiyzbzc93qTmRVe/J8=
 github.com/teambition/rrule-go v1.8.2/go.mod h1:Ieq5AbrKGciP1V//Wq8ktsTXwSwJHDD5mD/wLBGl3p4=
+github.com/ulule/limiter/v3 v3.11.2 h1:P4yOrxoEMJbOTfRJR2OzjL90oflzYPPmWg+dvwN2tHA=
+github.com/ulule/limiter/v3 v3.11.2/go.mod h1:QG5GnFOCV+k7lrL5Y8kgEeeflPH3+Cviqlqa8SVSQxI=
 github.com/unrolled/secure v1.13.0 h1:sdr3Phw2+f8Px8HE5sd1EHdj1aV3yUwed/uZXChLFsk=
 github.com/unrolled/secure v1.13.0/go.mod h1:BmF5hyM6tXczk3MpQkFf1hpKSRqCyhqcbiQtiAF7+40=
 github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
@@ -161,9 +158,6 @@ golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo=
 golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ=
-golang.org/x/time v0.0.0-20200416051211-89c76fbcd5d1/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
-golang.org/x/time v0.3.0 h1:rg5rLMjNzMS1RkNLzCG38eapWhnYLFYXDXj2gOlr8j4=
-golang.org/x/time v0.3.0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=