mirror of
https://github.com/getAlby/hub.git
synced 2026-08-13 12:33:39 +02:00
fix: prevent backup restore from writing outside the restore directory
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>
This commit is contained in:
parent
037765794d
commit
258c7b5602
2 changed files with 69 additions and 1 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue