From be0aaa70601af919f68fa1153f76bcf6335bc0b5 Mon Sep 17 00:00:00 2001 From: kolaente Date: Fri, 20 Mar 2026 11:41:16 +0100 Subject: [PATCH] fix: adapt image preview DoS protection to new FileStorage interface File.File is now io.ReadCloser (no Seek). Buffer the file bytes once via io.ReadAll, then use bytes.NewReader for both DecodeConfig and Decode. Test updated to use io.NopCloser instead of afero. --- pkg/models/task_attachment.go | 16 +++++++++------- pkg/models/task_attachment_test.go | 12 ++---------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/pkg/models/task_attachment.go b/pkg/models/task_attachment.go index 75c42186f..3f95f412a 100644 --- a/pkg/models/task_attachment.go +++ b/pkg/models/task_attachment.go @@ -222,10 +222,17 @@ func (ta *TaskAttachment) GetPreview(previewSize PreviewSize) []byte { cacheKey := cacheKeyForTaskAttachmentPreview(ta.ID, previewSize) result, err := keyvalue.Remember(cacheKey, func() (any, error) { + // Read all bytes up front so we can inspect dimensions without seeking. + // The file is an io.ReadCloser (no Seek), so we buffer it once. + data, err := io.ReadAll(ta.File.File) + if err != nil { + return nil, err + } + // Check image dimensions before full decode to prevent DoS // from decompression bombs (small file, huge pixel dimensions) const maxPixels = 50_000_000 // 50 megapixels - cfg, _, err := image.DecodeConfig(ta.File.File) + cfg, _, err := image.DecodeConfig(bytes.NewReader(data)) if err != nil { return nil, err } @@ -233,12 +240,7 @@ func (ta *TaskAttachment) GetPreview(previewSize PreviewSize) []byte { return nil, fmt.Errorf("image dimensions %dx%d exceed maximum of %d pixels", cfg.Width, cfg.Height, maxPixels) } - // Seek back to start for full decode - if _, err := ta.File.File.Seek(0, io.SeekStart); err != nil { - return nil, err - } - - img, _, err := image.Decode(ta.File.File) + img, _, err := image.Decode(bytes.NewReader(data)) if err != nil { return nil, err } diff --git a/pkg/models/task_attachment_test.go b/pkg/models/task_attachment_test.go index 73704563c..a2e06a365 100644 --- a/pkg/models/task_attachment_test.go +++ b/pkg/models/task_attachment_test.go @@ -20,6 +20,7 @@ import ( "bytes" "image" "image/png" + "io" "os" "testing" @@ -27,7 +28,6 @@ import ( "code.vikunja.io/api/pkg/files" "code.vikunja.io/api/pkg/user" - "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -195,18 +195,10 @@ func TestAttachmentPreviewRejectsLargeImages(t *testing.T) { err := png.Encode(&buf, img) require.NoError(t, err) - // Write the PNG to an in-memory afero filesystem so we get an afero.File - memFs := afero.NewMemMapFs() - err = afero.WriteFile(memFs, "large.png", buf.Bytes(), 0644) - require.NoError(t, err) - f, err := memFs.Open("large.png") - require.NoError(t, err) - defer f.Close() - attachment := &TaskAttachment{ ID: 999999, File: &files.File{ - File: f, + File: io.NopCloser(bytes.NewReader(buf.Bytes())), }, }