diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a776d8d..fe347154 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,13 @@ - Additional fileblob bucket escape checks for just in case if the filesystem abstraction is used as standalone without the PocketBase validations. +- Added a global restriction for the max `WxH` pixels size when generating a thumb to reduce the risk of excessive memory usage. + The issue was initially privately reported by [@reload3d](https://github.com/reload3d) but it is treated as regular public issue since it is a [known Go caveat](https://github.com/golang/go/issues/27830). + The default limit is pretty generous and currently set to max `250<<20 (262144000)` pixels and can be changed with the env `PB_THUMBS_MAX_SOURCE_PIXELS`. + **The workaround is not bulletproof and thumbs generation remains one of the main culprit for potential OOMs in PocketBase**, so better solutions will have to be investigated in the future, including eventually switching to [libvips](https://github.com/libvips/libvips) or other similar library once there is [better CGO portability](https://github.com/golang/go/issues/81450) _(if you have better suggestions, please open a discussion)_. + - Bumped the min Go GitHub action version to 1.27.3 as it includes a [security fix](https://groups.google.com/g/golang-announce/c/66pojueaQek) related to the recent eMudhra CA compromise. - _I didn't rush a fix 2 days ago as the impact for most PocketBase servers is not that serious and requires to already have a MiTM, but nonetheless it is recommended to update. If you can, check also for OS/Docker image updates to make sure that you have the latest `ca-certificates` bundle._ + _I didn't rush a fix 2 days ago as the impact for most PocketBase servers is not that serious and requires to already have a MiTM, but nonetheless it is recommended to update. Please make sure to also check for OS/Docker image updates to make sure that you have the latest `ca-certificates` bundle._ - Bumped `golang.org/x/` dependencies ([#7865](https://github.com/pocketbase/pocketbase/discussions/7865)). diff --git a/tools/filesystem/filesystem.go b/tools/filesystem/filesystem.go index 32185e95..7264cd89 100644 --- a/tools/filesystem/filesystem.go +++ b/tools/filesystem/filesystem.go @@ -3,6 +3,7 @@ package filesystem import ( "context" "errors" + "fmt" "image" "io" "mime/multipart" @@ -25,6 +26,7 @@ import ( "github.com/pocketbase/pocketbase/tools/hook" "github.com/pocketbase/pocketbase/tools/list" "github.com/pocketbase/pocketbase/tools/routine" + "github.com/spf13/cast" // manually register the webp decoder because disintegration/imaging does not support webp _ "golang.org/x/image/webp" @@ -284,7 +286,10 @@ func (s *System) UploadFile(file *File, fileKey string) error { } // rewind - f.Seek(0, io.SeekStart) + _, err = f.Seek(0, io.SeekStart) + if err != nil { + return fmt.Errorf("failed to rewind: %w", err) + } originalName := file.OriginalName if len(originalName) > 255 { @@ -326,7 +331,10 @@ func (s *System) UploadMultipart(fh *multipart.FileHeader, fileKey string) error } // rewind - f.Seek(0, io.SeekStart) + _, err = f.Seek(0, io.SeekStart) + if err != nil { + return fmt.Errorf("failed to rewind: %w", err) + } originalName := fh.Filename if len(originalName) > 255 { @@ -638,6 +646,28 @@ func (s *System) createThumb(originalKey, thumbKey, thumbSize string) error { } defer r.Close() + // check image dimensions to minimize excessive memory usage + config, _, err := image.DecodeConfig(r) + if err != nil { + return fmt.Errorf("image headers decoding failed: %w", err) + } + + maxPixels := cast.ToInt64(os.Getenv("PB_THUMBS_MAX_SOURCE_PIXELS")) + if maxPixels <= 0 { + maxPixels = 250 << 20 + } + + currentDimensions := int64(config.Width) * int64(config.Height) + if currentDimensions > maxPixels { + return fmt.Errorf("the original image is too large (%dx%d) and exceeds the total max allowed %d pixels", config.Width, config.Height, maxPixels) + } + + // rewind + _, err = r.Seek(0, io.SeekStart) + if err != nil { + return fmt.Errorf("failed to rewind reader after config decode: %w", err) + } + // create imaging object from the original reader // (note: only the first frame for animated image formats) img, decodeErr := imaging.Decode(r, imaging.AutoOrientation(true)) diff --git a/tools/filesystem/filesystem_test.go b/tools/filesystem/filesystem_test.go index c470713e..673d7ae3 100644 --- a/tools/filesystem/filesystem_test.go +++ b/tools/filesystem/filesystem_test.go @@ -12,6 +12,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strconv" "strings" "testing" @@ -1090,6 +1091,44 @@ func TestFilesystemCreateThumb(t *testing.T) { } } +func TestFilesystemCreateThumbWithMaxPixelsLimit(t *testing.T) { + dir := createTestDir(t) + defer os.RemoveAll(dir) + + fsys, err := filesystem.NewLocal(dir) + if err != nil { + t.Fatal(err) + } + defer fsys.Close() + + scenarios := []struct { + name string + file string + thumb string + size string + maxPixels int + expectErr bool + }{ + {"default limit", "image.png", "thumb_WxH", "100x100", 0, false}, + {"below limit", "image.png", "thumb_WxH", "100x100", 24, true}, + {"exact limit", "image.png", "thumb_WxH", "100x100", 25, false}, + {"above limit", "image.png", "thumb_WxH", "100x100", 26, false}, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + os.Setenv("PB_THUMBS_MAX_SOURCE_PIXELS", strconv.Itoa(s.maxPixels)) + + err = fsys.CreateThumb(s.file, s.thumb, s.size) + + hasErr := err != nil + if hasErr != s.expectErr { + t.Fatalf("Expected hasErr to be %v, got %v (%v)", s.expectErr, hasErr, err) + } + }) + } +} + func TestFilesystemClose(t *testing.T) { dir := createTestDir(t) defer os.RemoveAll(dir) @@ -1157,7 +1196,7 @@ func createTestDir(t *testing.T) string { if err != nil { t.Fatal(err) } - imgRect := image.Rect(0, 0, 1, 1) // tiny 1x1 png + imgRect := image.Rect(0, 0, 5, 5) // tiny 5x5 png _ = png.Encode(file, imgRect) file.Close() err = os.WriteFile(filepath.Join(dir, "image.png.attrs"), []byte(`{"user.cache_control":"","user.content_disposition":"","user.content_encoding":"","user.content_language":"","user.content_type":"image/png","user.metadata":null}`), 0644) @@ -1172,7 +1211,7 @@ func createTestDir(t *testing.T) string { if err != nil { t.Fatal(err) } - imgRect := image.Rect(0, 0, 1, 1) // tiny 1x1 jpg + imgRect := image.Rect(0, 0, 5, 5) // tiny 5x5 jpg _ = jpeg.Encode(file, imgRect, nil) file.Close() err = os.WriteFile(filepath.Join(dir, "image.jpg.attrs"), []byte(`{"user.cache_control":"","user.content_disposition":"","user.content_encoding":"","user.content_language":"","user.content_type":"image/jpeg","user.metadata":null}`), 0644) @@ -1208,7 +1247,7 @@ func createTestDir(t *testing.T) string { if err != nil { t.Fatal(err) } - imgRect := image.Rect(0, 0, 1, 1) // tiny 1x1 png + imgRect := image.Rect(0, 0, 5, 5) // tiny 5x5 png _ = png.Encode(file, imgRect) file.Close() } @@ -1220,7 +1259,7 @@ func createTestDir(t *testing.T) string { if err != nil { t.Fatal(err) } - imgRect := image.Rect(0, 0, 1, 1) // tiny 1x1 jpg + imgRect := image.Rect(0, 0, 5, 5) // tiny 5x5 jpg _ = jpeg.Encode(file, imgRect, nil) file.Close() err = os.WriteFile(fullPath+".attrs", []byte(`{"user.cache_control":"","user.content_disposition":"","user.content_encoding":"","user.content_language":"","user.content_type":"image/jpeg","user.metadata":null}`), 0644)