security: close thumbnail and upload quota bypasses

internal/api/review_b_audit_test.go

@@ -1,9 +1,14 @@
 package api
 
 import (
+	"bytes"
+	"encoding/binary"
+	"hash/crc32"
 	"image"
 	"image/color"
 	"image/png"
+	"mime/multipart"
+	"net/http"
 	"net/http/httptest"
 	"os"
 	"path/filepath"
@@ -104,3 +109,73 @@ func TestAuditThumbnailPixelLimit(t *testing.T) {
 		t.Fatalf("thumbnail pixel-limit status=%d want 415", rr.Code)
 	}
 }
+
+func TestAuditThumbnailRejectsDeclaredHugeDimensionsBeforeDecode(t *testing.T) {
+	root := t.TempDir()
+	f, err := os.Create(filepath.Join(root, "bomb.png"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	img := image.NewRGBA(image.Rect(0, 0, 1, 1))
+	if err := png.Encode(f, img); err != nil {
+		f.Close()
+		t.Fatal(err)
+	}
+	if err := f.Close(); err != nil {
+		t.Fatal(err)
+	}
+	data, err := os.ReadFile(filepath.Join(root, "bomb.png"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	// PNG IHDR starts at byte 16. Replace the 1x1 declaration with a
+	// deliberately huge image while keeping the file otherwise tiny/valid.
+	binary.BigEndian.PutUint32(data[16:20], 100000)
+	binary.BigEndian.PutUint32(data[20:24], 100000)
+	crc := crc32.ChecksumIEEE(data[12:29])
+	binary.BigEndian.PutUint32(data[29:33], crc)
+	if err := os.WriteFile(filepath.Join(root, "bomb.png"), data, 0600); err != nil {
+		t.Fatal(err)
+	}
+	s := &Server{Store: storage.Local{Root: root}, Auth: &auth.Manager{Guest: &auth.Account{Mask: auth.Read}}, ImagePreview: true, ThumbnailMaxPixels: 40_000_000}
+	rr := httptest.NewRecorder()
+	req := httptest.NewRequest("GET", "/chfs/thumbnail?file=bomb.png", nil)
+	s.thumbnail(rr, req)
+	if rr.Code != 415 {
+		t.Fatalf("thumbnail bomb status=%d want 415", rr.Code)
+	}
+}
+
+func TestAuditUploadMaxBytesReaderBeforeMultipartSpool(t *testing.T) {
+	for _, tc := range []struct {
+		name    string
+		handler func(*Server, http.ResponseWriter, *http.Request)
+	}{
+		{"upload", (*Server).upload},
+		{"upload2", (*Server).upload2},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			root := t.TempDir()
+			var body bytes.Buffer
+			mw := multipart.NewWriter(&body)
+			part, err := mw.CreateFormFile("file", "oversize.bin")
+			if err != nil {
+				t.Fatal(err)
+			}
+			if _, err := part.Write(make([]byte, 1024)); err != nil {
+				t.Fatal(err)
+			}
+			if err := mw.Close(); err != nil {
+				t.Fatal(err)
+			}
+			req := httptest.NewRequest(http.MethodPost, "/chfs/"+tc.name+"?folder=", &body)
+			req.Header.Set("Content-Type", mw.FormDataContentType())
+			s := &Server{Store: storage.Local{Root: root}, Auth: &auth.Manager{Guest: &auth.Account{Mask: auth.Write}}, UploadMaxTotalSize: 128}
+			rr := httptest.NewRecorder()
+			tc.handler(s, rr, req)
+			if rr.Code != http.StatusRequestEntityTooLarge {
+				t.Fatalf("status=%d want 413", rr.Code)
+			}
+		})
+	}
+}

internal/api/server.go

@@ -849,8 +849,15 @@ func (s *Server) updatetext(w http.ResponseWriter, r *http.Request) {
 	writeCreated(w)
 }
 func (s *Server) upload(w http.ResponseWriter, r *http.Request) {
+	if s.UploadMaxTotalSize > 0 {
+		r.Body = http.MaxBytesReader(w, r.Body, s.UploadMaxTotalSize)
+	}
 	if e := r.ParseMultipartForm(64 << 20); e != nil {
-		http.Error(w, e.Error(), 400)
+		if _, ok := e.(*http.MaxBytesError); ok {
+			http.Error(w, e.Error(), http.StatusRequestEntityTooLarge)
+		} else {
+			http.Error(w, e.Error(), http.StatusBadRequest)
+		}
 		return
 	}
 	a := account(r, s)
@@ -974,6 +981,9 @@ func savePart(st storage.Storage, target string, h *multipart.FileHeader, maxSiz
 }
 
 func (s *Server) upload2(w http.ResponseWriter, r *http.Request) {
+	if s.UploadMaxTotalSize > 0 {
+		r.Body = http.MaxBytesReader(w, r.Body, s.UploadMaxTotalSize)
+	}
 	a := account(r, s)
 	rawDir := r.URL.Query().Get("folder")
 	if unsafePath(rawDir) {
@@ -986,7 +996,11 @@ func (s *Server) upload2(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	if err := r.ParseMultipartForm(128 << 20); err != nil {
-		http.Error(w, err.Error(), 400)
+		if _, ok := err.(*http.MaxBytesError); ok {
+			http.Error(w, err.Error(), http.StatusRequestEntityTooLarge)
+		} else {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+		}
 		return
 	}
 	// Preflight the complete multipart batch before creating anything.
@@ -1258,15 +1272,19 @@ func (s *Server) thumbnail(w http.ResponseWriter, r *http.Request) {
 		}
 	})
 	data, err := s.ThumbnailCache.Do(key, func() ([]byte, error) {
-		src, _, err := image.Decode(f)
+		config, _, err := image.DecodeConfig(f)
 		if err != nil {
 			return nil, err
 		}
-		if s.ThumbnailMaxPixels > 0 {
-			b := src.Bounds()
-			if int64(b.Dx())*int64(b.Dy()) > s.ThumbnailMaxPixels {
-				return nil, fmt.Errorf("thumbnail dimensions too large")
-			}
+		if s.ThumbnailMaxPixels > 0 && int64(config.Width)*int64(config.Height) > s.ThumbnailMaxPixels {
+			return nil, fmt.Errorf("thumbnail dimensions too large")
+		}
+		if _, err := f.Seek(0, io.SeekStart); err != nil {
+			return nil, err
+		}
+		src, _, err := image.Decode(f)
+		if err != nil {
+			return nil, err
 		}
 		thumb := transform.Resize(src, 320, 240, transform.Linear)
 		var buf bytes.Buffer