From 33389bb0b35f16080f3bb25459f3e386eb62bb3c Mon Sep 17 00:00:00 2001 From: kolaente Date: Thu, 9 Apr 2026 17:15:44 +0200 Subject: [PATCH] test(migration): regression test for forged attachment size Builds an in-memory export zip with a 2 MB payload and a data.json that claims size: 0, then asserts neither the honest 2 MB row nor the forged 0-size row ends up in the files table. Covers GHSA-qh78-rvg3-cv54. --- .../migration/vikunja-file/vikunja_test.go | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/pkg/modules/migration/vikunja-file/vikunja_test.go b/pkg/modules/migration/vikunja-file/vikunja_test.go index 848f568b7..bd07b8a2e 100644 --- a/pkg/modules/migration/vikunja-file/vikunja_test.go +++ b/pkg/modules/migration/vikunja-file/vikunja_test.go @@ -17,9 +17,12 @@ package vikunjafile import ( + "archive/zip" + "bytes" "os" "testing" + "code.vikunja.io/api/pkg/config" "code.vikunja.io/api/pkg/db" "code.vikunja.io/api/pkg/user" @@ -74,6 +77,76 @@ func TestVikunjaFileMigrator_Migrate(t *testing.T) { "created_by_id": u.ID, }, false) }) + t.Run("rejects attachment with forged size smaller than content", func(t *testing.T) { + // Regression: GHSA-qh78-rvg3-cv54. Zip entry carries 2 MB of + // content but data.json claims size: 0, bypassing the 1 MB cap. + db.LoadAndAssertFixtures(t) + + oldMax := config.FilesMaxSize.GetString() + config.FilesMaxSize.Set("1MB") + defer func() { + config.FilesMaxSize.Set(oldMax) + _ = config.SetMaxFileSizeMBytesFromString(oldMax) + }() + require.NoError(t, config.SetMaxFileSizeMBytesFromString("1MB")) + + payload := make([]byte, 2*1024*1024) + + dataJSON := `[{ + "id": 1, + "title": "evil project", + "tasks": [{ + "id": 1, + "title": "evil task", + "attachments": [{ + "id": 1, + "file": {"id": 1, "name": "evil.bin", "size": 0} + }] + }], + "views": [] + }]` + + var zipBuf bytes.Buffer + zw := zip.NewWriter(&zipBuf) + + vf, err := zw.Create("VERSION") + require.NoError(t, err) + _, err = vf.Write([]byte("dev")) + require.NoError(t, err) + + df, err := zw.Create("data.json") + require.NoError(t, err) + _, err = df.Write([]byte(dataJSON)) + require.NoError(t, err) + + ff, err := zw.Create("files/1") + require.NoError(t, err) + _, err = ff.Write(payload) + require.NoError(t, err) + + require.NoError(t, zw.Close()) + + m := &FileMigrator{} + u := &user.User{ID: 1} + + reader := bytes.NewReader(zipBuf.Bytes()) + err = m.Migrate(u, reader, int64(reader.Len())) + + // create_from_structure.go skips the oversized attachment via + // `continue`, so Migrate succeeds for the rest of the project. + require.NoError(t, err) + + // Forged 0-size row was the pre-fix outcome; assert neither + // size ends up persisted. + db.AssertMissing(t, "files", map[string]interface{}{ + "name": "evil.bin", + "size": 2 * 1024 * 1024, + }) + db.AssertMissing(t, "files", map[string]interface{}{ + "name": "evil.bin", + "size": 0, + }) + }) t.Run("should not accept an old import", func(t *testing.T) { db.LoadAndAssertFixtures(t)