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())), }, }