fix(files): derive file size from reader at creation boundary

Authoritative size now comes from the reader instead of the caller's
claim in CreateWithMimeAndSession. The migration import path accepts
attacker-controlled metadata (GHSA-qh78-rvg3-cv54), so trusting
realsize for the limit check allowed oversized uploads to be accepted
and stored.

measureReaderSize leaves the reader seeked to 0 so the measured value
matches the bytes storage backends will actually write.

pkg/files/files.go

@@ -20,6 +20,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"math"
 	"os"
 	"strconv"
 	"time"
@@ -114,15 +115,59 @@ func CreateWithMime(f io.ReadSeeker, realname string, realsize uint64, a web.Aut
 	return file, s.Commit()
 }
 
+// measureReaderSize returns the byte length of r and leaves r seeked to
+// 0, so the size we check matches what storage backends will write
+// (they also Seek(0, io.SeekStart) before reading).
+func measureReaderSize(r io.ReadSeeker) (uint64, error) {
+	if _, err := r.Seek(0, io.SeekStart); err != nil {
+		return 0, err
+	}
+	end, err := r.Seek(0, io.SeekEnd)
+	if err != nil {
+		return 0, err
+	}
+	if _, err := r.Seek(0, io.SeekStart); err != nil {
+		return 0, err
+	}
+	if end < 0 {
+		return 0, fmt.Errorf("reader end %d is negative", end)
+	}
+	return uint64(end), nil
+}
+
 func CreateWithMimeAndSession(s *xorm.Session, f io.ReadSeeker, realname string, realsize uint64, a web.Auth, mime string, checkFileSizeLimit bool) (file *File, err error) {
-	if realsize > config.GetMaxFileSizeInMBytes()*uint64(datasize.MB) && checkFileSizeLimit {
-		return nil, ErrFileIsTooLarge{Size: realsize}
+	// Authoritative size comes from the reader, not the caller: the
+	// migration import path accepts attacker-controlled metadata
+	// (GHSA-qh78-rvg3-cv54) and several other callers pass stale values.
+	measured, mErr := measureReaderSize(f)
+	if mErr != nil {
+		return nil, fmt.Errorf("failed to measure file size: %w", mErr)
+	}
+
+	if checkFileSizeLimit {
+		// Overflow-safe: exabyte-range configs would wrap uint64.
+		maxMB := config.GetMaxFileSizeInMBytes()
+		var maxBytes uint64
+		if maxMB > math.MaxUint64/uint64(datasize.MB) {
+			maxBytes = math.MaxUint64
+		} else {
+			maxBytes = maxMB * uint64(datasize.MB)
+		}
+		if measured > maxBytes {
+			return nil, ErrFileIsTooLarge{Size: measured}
+		}
+	}
+
+	// Surface buggy callers that lie about size.
+	if realsize != 0 && realsize != measured {
+		log.Debugf("files.Create: caller-supplied size %d does not match measured size %d for %q",
+			realsize, measured, realname)
 	}
 
 	// We first insert the file into the db to get it's ID
 	file = &File{
 		Name:        realname,
-		Size:        realsize,
+		Size:        measured,
 		CreatedByID: a.GetID(),
 		Mime:        mime,
 	}

pkg/files/files_test.go

@@ -24,6 +24,7 @@ import (
 	"os"
 	"testing"
 
+	"code.vikunja.io/api/pkg/config"
 	"code.vikunja.io/api/pkg/db"
 
 	"github.com/stretchr/testify/assert"
@@ -42,7 +43,8 @@ func TestCreate(t *testing.T) {
 	t.Run("Normal", func(t *testing.T) {
 		initFixtures(t)
 		ta := &testauth{id: 1}
-		createdFile, err := Create(bytes.NewReader([]byte("testfile")), "testfile", 100, ta)
+		content := []byte("testfile")
+		createdFile, err := Create(bytes.NewReader(content), "testfile", uint64(len(content)), ta)
 		require.NoError(t, err)
 
 		// Check the file was created correctly
@@ -51,18 +53,67 @@ func TestCreate(t *testing.T) {
 		require.NoError(t, err)
 		assert.Equal(t, int64(1), file.CreatedByID)
 		assert.Equal(t, "testfile", file.Name)
-		assert.Equal(t, uint64(100), file.Size)
+		assert.Equal(t, uint64(len(content)), file.Size)
 
 	})
 	t.Run("Too Large", func(t *testing.T) {
 		initFixtures(t)
+		// initFixtures resets FilesMaxSize, so override afterwards.
+		oldMax := config.FilesMaxSize.GetString()
+		require.NoError(t, config.SetMaxFileSizeMBytesFromString("1MB"))
+		t.Cleanup(func() { _ = config.SetMaxFileSizeMBytesFromString(oldMax) })
+
 		ta := &testauth{id: 1}
-		_, err := Create(bytes.NewReader([]byte("testfile")), "testfile", 99999999999, ta)
+		_, err := Create(bytes.NewReader(make([]byte, 2*1024*1024)), "big.bin", 0, ta)
 		require.Error(t, err)
 		assert.True(t, IsErrFileIsTooLarge(err))
 	})
 }
 
+func TestCreate_HardenedAgainstLyingCaller(t *testing.T) {
+	// Regression: GHSA-qh78-rvg3-cv54. Caller's claimed size must not
+	// influence the limit check or the stored size.
+
+	// setLimit must run AFTER initFixtures since initFixtures resets it.
+	setLimit := func(t *testing.T, size string) {
+		t.Helper()
+		oldMax := config.FilesMaxSize.GetString()
+		require.NoError(t, config.SetMaxFileSizeMBytesFromString(size))
+		t.Cleanup(func() {
+			_ = config.SetMaxFileSizeMBytesFromString(oldMax)
+		})
+	}
+
+	ta := &testauth{id: 1}
+
+	t.Run("content over limit, claimed size under limit", func(t *testing.T) {
+		initFixtures(t)
+		setLimit(t, "1MB")
+		payload := make([]byte, 2*1024*1024)
+		_, err := Create(bytes.NewReader(payload), "sneaky.bin", 1, ta)
+		require.Error(t, err)
+		assert.True(t, IsErrFileIsTooLarge(err))
+	})
+
+	t.Run("content under limit, claimed size over limit", func(t *testing.T) {
+		initFixtures(t)
+		setLimit(t, "1MB")
+		payload := []byte("hi")
+		f, err := Create(bytes.NewReader(payload), "honest.txt", 99999999999, ta)
+		require.NoError(t, err)
+		assert.Equal(t, uint64(len(payload)), f.Size)
+	})
+
+	t.Run("content exactly at limit", func(t *testing.T) {
+		initFixtures(t)
+		setLimit(t, "1MB")
+		payload := make([]byte, 1*1024*1024)
+		f, err := Create(bytes.NewReader(payload), "edge.bin", 0, ta)
+		require.NoError(t, err)
+		assert.Equal(t, uint64(1*1024*1024), f.Size)
+	})
+}
+
 func TestCreateDetectsMimeType(t *testing.T) {
 	initFixtures(t)
 	ta := &testauth{id: 1}

pkg/models/task_attachment_test.go

@@ -104,7 +104,8 @@ func TestTaskAttachment_NewAttachment(t *testing.T) {
 	}
 	testuser := &user.User{ID: 1}
 
-	err := ta.NewAttachment(s, bytes.NewReader([]byte("testingstuff")), "testfile", 100, testuser)
+	content := []byte("testingstuff")
+	err := ta.NewAttachment(s, bytes.NewReader(content), "testfile", uint64(len(content)), testuser)
 	require.NoError(t, err)
 	assert.NotEqual(t, 0, ta.FileID)
 	_, err = files.FileStat(ta.File)
@@ -122,7 +123,7 @@ func TestTaskAttachment_NewAttachment(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, testuser.ID, ta.File.CreatedByID)
 	assert.Equal(t, "testfile", ta.File.Name)
-	assert.Equal(t, uint64(100), ta.File.Size)
+	assert.Equal(t, uint64(len(content)), ta.File.Size)
 	assert.NotEmpty(t, ta.File.Mime, "mime type should be detected and stored")
 
 	// Extra test for max size test