From fa5cc3511e5f0507d6de67644f4bbf28b002088c Mon Sep 17 00:00:00 2001 From: Roland <33993199+rolznz@users.noreply.github.com> Date: Mon, 10 Aug 2026 23:42:45 +0700 Subject: [PATCH] fix: prevent backup restore from writing outside the restore directory (#2529) Archive entry names come from the uploaded backup and were joined to the restore directory without validation, so an entry name containing ".." segments could resolve to a path outside it. Reject entries whose name is absolute or escapes the restore directory, and confirm the cleaned destination path stays within it before writing. Add a test covering rejection of an entry that points outside the restore directory. Co-authored-by: Claude Fable 5 --- api/backup.go | 18 +++++++++++++++- api/backup_test.go | 52 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/api/backup.go b/api/backup.go index f8f6b0fc..9d2d7622 100644 --- a/api/backup.go +++ b/api/backup.go @@ -257,8 +257,24 @@ func (api *api) RestoreBackup(unlockPassword string, r io.Reader) error { return fmt.Errorf("failed to create zip reader: %w", err) } + restoreDir := filepath.Join(workDir, "restore") + extractZipEntry := func(zipFile *zip.File) error { - fsFilePath := filepath.Join(workDir, "restore", filepath.FromSlash(zipFile.Name)) + // Entry names come from the archive and must not be trusted. Reject any + // name that is absolute or points outside the restore directory via + // ".." segments before joining it to a path. + entryName := filepath.FromSlash(zipFile.Name) + if !filepath.IsLocal(entryName) { + return fmt.Errorf("refusing to extract zip entry outside restore directory: %q", zipFile.Name) + } + + fsFilePath := filepath.Join(restoreDir, entryName) + + // Confirm the cleaned path is still contained within the restore + // directory. + if fsFilePath != restoreDir && !strings.HasPrefix(fsFilePath, restoreDir+string(os.PathSeparator)) { + return fmt.Errorf("refusing to extract zip entry outside restore directory: %q", zipFile.Name) + } if err = os.MkdirAll(filepath.Dir(fsFilePath), 0700); err != nil { return fmt.Errorf("failed to create directory for zip entry: %w", err) diff --git a/api/backup_test.go b/api/backup_test.go index b41eea19..784367f9 100644 --- a/api/backup_test.go +++ b/api/backup_test.go @@ -104,3 +104,55 @@ func TestCreateBackup(t *testing.T) { require.Equal(t, app.Name, restoredApp.Name) require.Equal(t, app.AppPubkey, restoredApp.AppPubkey) } + +// TestRestoreBackupRejectsPathTraversal verifies that a backup archive +// containing an entry whose name points outside the restore directory is +// rejected and that no file is written outside it. +func TestRestoreBackupRejectsPathTraversal(t *testing.T) { + logger.Init(strconv.Itoa(int(logrus.DebugLevel))) + + gormDB, err := test_db.NewDB(t) + require.NoError(t, err) + defer test_db.CloseDB(gormDB) + + if gormDB.Dialector.Name() != "sqlite" { + t.Skip("restore is only supported on sqlite") + } + + workDir := t.TempDir() + + appConfig := &config.AppConfig{ + Workdir: workDir, + DatabaseUri: test_db.GetTestDatabaseURI(), + } + cfg, err := config.NewConfig(appConfig, gormDB) + require.NoError(t, err) + + theAPI := &api{ + db: gormDB, + cfg: cfg, + } + + unlockPassword := "" + + // The restore directory is /restore, so a "../" entry targets a + // file directly in the working directory, one level above it. + const escapeEntryName = "../pwned.txt" + escapeTarget := filepath.Join(workDir, "pwned.txt") + + var buf bytes.Buffer + cw, err := encryptingWriter(&buf, unlockPassword) + require.NoError(t, err) + zw := zip.NewWriter(cw) + entryWriter, err := zw.Create(escapeEntryName) + require.NoError(t, err) + _, err = entryWriter.Write([]byte("pwned")) + require.NoError(t, err) + require.NoError(t, zw.Close()) + + err = theAPI.RestoreBackup(unlockPassword, &buf) + require.Error(t, err) + + _, statErr := os.Stat(escapeTarget) + require.True(t, os.IsNotExist(statErr), "traversal entry must not be written outside the restore directory") +}