Răsfoiți Sursa

feat(web): cap request body size on state-changing routes (#5271)

* feat(web): cap request body size on state-changing routes

* fix(web): exempt importDB from request body size cap

The 10 MiB body cap was applied globally, which would break database
restore (/panel/api/server/importDB) on any panel whose SQLite backup
exceeds the limit. Make MaxBodyBytes accept exempt path suffixes and
pass importDB through uncapped; the cap still covers all other
state-changing routes. Add a test for the skip-suffix behavior.

---------

Co-authored-by: Sanaei <[email protected]>
n0ctal 16 ore în urmă
părinte
comite
71616b7cf2

+ 43 - 0
internal/web/middleware/bodylimit.go

@@ -0,0 +1,43 @@
+package middleware
+
+import (
+	"net/http"
+	"strings"
+
+	"github.com/gin-gonic/gin"
+)
+
+// MaxBodyBytes caps the request body size for state-changing requests. It wraps
+// the body in an http.MaxBytesReader so that any handler reading it (gin's
+// ShouldBind, manual io.ReadAll, etc.) receives an error once the limit is
+// exceeded, which the existing bind-failure path reports as a 400 rather than
+// allocating an unbounded buffer or starting a long DB transaction.
+//
+// Methods without a body (GET/HEAD/OPTIONS/TRACE) and a non-positive limit are
+// passed through untouched. Paths ending in one of skipSuffixes are also passed
+// through uncapped — these are routes that legitimately accept a large upload
+// (e.g. database restore, which streams a multi-MiB SQLite file).
+func MaxBodyBytes(limit int64, skipSuffixes ...string) gin.HandlerFunc {
+	return func(c *gin.Context) {
+		if limit > 0 {
+			switch c.Request.Method {
+			case http.MethodGet, http.MethodHead, http.MethodOptions, http.MethodTrace:
+			default:
+				if c.Request.Body != nil && !hasSuffix(c.Request.URL.Path, skipSuffixes) {
+					c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, limit)
+				}
+			}
+		}
+		c.Next()
+	}
+}
+
+// hasSuffix reports whether path ends in any of the given suffixes.
+func hasSuffix(path string, suffixes []string) bool {
+	for _, s := range suffixes {
+		if strings.HasSuffix(path, s) {
+			return true
+		}
+	}
+	return false
+}

+ 80 - 0
internal/web/middleware/bodylimit_test.go

@@ -0,0 +1,80 @@
+package middleware
+
+import (
+	"bytes"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/gin-gonic/gin"
+)
+
+func TestMaxBodyBytes(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	const limit = 16
+
+	r := gin.New()
+	r.Use(MaxBodyBytes(limit))
+	r.POST("/x", func(c *gin.Context) {
+		if _, err := io.ReadAll(c.Request.Body); err != nil {
+			c.String(http.StatusRequestEntityTooLarge, "too big")
+			return
+		}
+		c.String(http.StatusOK, "ok")
+	})
+	r.GET("/x", func(c *gin.Context) { c.String(http.StatusOK, "ok") })
+
+	// Body within the limit is read normally.
+	w := httptest.NewRecorder()
+	r.ServeHTTP(w, httptest.NewRequest(http.MethodPost, "/x", strings.NewReader("0123456789")))
+	if w.Code != http.StatusOK {
+		t.Errorf("under-limit POST: got %d, want 200", w.Code)
+	}
+
+	// Body over the limit makes the handler's read fail (no unbounded buffer).
+	w = httptest.NewRecorder()
+	r.ServeHTTP(w, httptest.NewRequest(http.MethodPost, "/x", bytes.NewReader(make([]byte, limit*4))))
+	if w.Code == http.StatusOK {
+		t.Errorf("over-limit POST should not succeed, got 200")
+	}
+
+	// Bodyless methods pass through untouched.
+	w = httptest.NewRecorder()
+	r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/x", nil))
+	if w.Code != http.StatusOK {
+		t.Errorf("GET should pass through, got %d", w.Code)
+	}
+}
+
+func TestMaxBodyBytesSkipSuffix(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	const limit = 16
+
+	r := gin.New()
+	r.Use(MaxBodyBytes(limit, "/server/importDB"))
+	read := func(c *gin.Context) {
+		if _, err := io.ReadAll(c.Request.Body); err != nil {
+			c.String(http.StatusRequestEntityTooLarge, "too big")
+			return
+		}
+		c.String(http.StatusOK, "ok")
+	}
+	r.POST("/server/importDB", read)
+	r.POST("/x", read)
+
+	// Exempt route reads an over-limit body without error.
+	w := httptest.NewRecorder()
+	r.ServeHTTP(w, httptest.NewRequest(http.MethodPost, "/server/importDB", bytes.NewReader(make([]byte, limit*4))))
+	if w.Code != http.StatusOK {
+		t.Errorf("exempt route should pass through over-limit body, got %d", w.Code)
+	}
+
+	// Non-exempt route is still capped.
+	w = httptest.NewRecorder()
+	r.ServeHTTP(w, httptest.NewRequest(http.MethodPost, "/x", bytes.NewReader(make([]byte, limit*4))))
+	if w.Code == http.StatusOK {
+		t.Errorf("non-exempt over-limit POST should not succeed, got 200")
+	}
+}

+ 9 - 0
internal/web/web.go

@@ -153,6 +153,15 @@ func (s *Server) initRouter() (*gin.Engine, error) {
 	sendHSTS := directHTTPS && !config.IsSkipHSTS()
 	engine.Use(middleware.SecurityHeadersMiddleware(sendHSTS))
 
+	// Cap request bodies on state-changing requests so a stolen session/API
+	// token or a buggy client can't force large allocations or long DB
+	// transactions via bulk create/attach/import endpoints. GET/HEAD/OPTIONS
+	// carry no body and are left untouched. importDB restores a full SQLite
+	// backup that legitimately exceeds the cap, so it's exempt. Follow-up: make
+	// the limit a setting.
+	const maxRequestBodyBytes = 10 << 20 // 10 MiB
+	engine.Use(middleware.MaxBodyBytes(maxRequestBodyBytes, "/panel/api/server/importDB"))
+
 	webDomain, err := s.settingService.GetWebDomain()
 	if err != nil {
 		return nil, err