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 <noreply@anthropic.com>

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)

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 <workDir>/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")
+}