mirror of
https://github.com/pocketbase/pocketbase.git
synced 2026-10-23 14:30:26 +02:00
added extra check for the max original image dimension before generating thumb to minimize the risk of OOMs
This commit is contained in:
1 parent
5f49104aa4
commit
2c961da382
3 files changed
+81
-7
No files matched your search
+6
-1
@@ -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)).
|
||||
|
||||
|
||||
@@ -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))
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in new issue
Block a user