diff --git a/go/vt/mysqlctl/builtinbackupengine.go b/go/vt/mysqlctl/builtinbackupengine.go index 274c34aeeb4..3fa9b0b8381 100644 --- a/go/vt/mysqlctl/builtinbackupengine.go +++ b/go/vt/mysqlctl/builtinbackupengine.go @@ -36,6 +36,7 @@ import ( "github.com/spf13/pflag" "golang.org/x/sync/semaphore" + "vitess.io/vitess/go/fileutil" "vitess.io/vitess/go/ioutil" "vitess.io/vitess/go/mysql" "vitess.io/vitess/go/mysql/replication" @@ -167,7 +168,9 @@ func registerBuiltinBackupEngineFlags(fs *pflag.FlagSet) { fs.StringVar(&builtinIncrementalRestorePath, "builtinbackup-incremental-restore-path", builtinIncrementalRestorePath, "the directory where incremental restore files, namely binlog files, are extracted to. In k8s environments, this should be set to a directory that is shared between the vttablet and mysqld pods. The path should exist. When empty, the default OS temp dir is assumed.") } -// fullPath returns the full path of the entry, based on its type +// fullPath returns the full path of the entry, based on its type. +// It validates that the resolved path does not escape the base directory +// via path traversal (e.g. "../../" sequences in fe.Name). func (fe *FileEntry) fullPath(cnf *Mycnf) (string, error) { // find the root to use var root string @@ -184,7 +187,7 @@ func (fe *FileEntry) fullPath(cnf *Mycnf) (string, error) { return "", vterrors.Errorf(vtrpc.Code_UNKNOWN, "unknown base: %v", fe.Base) } - return path.Join(fe.ParentPath, root, fe.Name), nil + return fileutil.SafePathJoin(path.Join(fe.ParentPath, root), fe.Name) } // open attempts t oopen the file diff --git a/go/vt/mysqlctl/builtinbackupengine_test.go b/go/vt/mysqlctl/builtinbackupengine_test.go index 39e4aa7ae1c..61b1abdf3c2 100644 --- a/go/vt/mysqlctl/builtinbackupengine_test.go +++ b/go/vt/mysqlctl/builtinbackupengine_test.go @@ -21,7 +21,9 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "vitess.io/vitess/go/fileutil" tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata" ) @@ -70,6 +72,93 @@ func TestGetIncrementalFromPosGTIDSet(t *testing.T) { } } +func TestFileEntryFullPath(t *testing.T) { + cnf := &Mycnf{ + DataDir: "/vt/data", + InnodbDataHomeDir: "/vt/innodb-data", + InnodbLogGroupHomeDir: "/vt/innodb-log", + BinLogPath: "/vt/binlogs/mysql-bin", + } + + tests := []struct { + name string + entry FileEntry + wantPath string + wantError error + }{ + { + name: "valid relative path in DataDir", + entry: FileEntry{Base: backupData, Name: "mydb/table1.ibd"}, + wantPath: "/vt/data/mydb/table1.ibd", + }, + { + name: "valid relative path in InnodbDataHomeDir", + entry: FileEntry{Base: backupInnodbDataHomeDir, Name: "ibdata1"}, + wantPath: "/vt/innodb-data/ibdata1", + }, + { + name: "valid relative path in InnodbLogGroupHomeDir", + entry: FileEntry{Base: backupInnodbLogGroupHomeDir, Name: "ib_logfile0"}, + wantPath: "/vt/innodb-log/ib_logfile0", + }, + { + name: "valid relative path in BinlogDir", + entry: FileEntry{Base: backupBinlogDir, Name: "mysql-bin.000001"}, + wantPath: "/vt/binlogs/mysql-bin.000001", + }, + { + name: "valid path with ParentPath", + entry: FileEntry{Base: backupData, Name: "mydb/table1.ibd", ParentPath: "/tmp/restore"}, + wantPath: "/tmp/restore/vt/data/mydb/table1.ibd", + }, + { + name: "path traversal escapes base directory", + entry: FileEntry{Base: backupData, Name: "../../etc/passwd"}, + wantError: fileutil.ErrInvalidJoinedPath, + }, + { + name: "path traversal with deeper nesting", + entry: FileEntry{Base: backupData, Name: "mydb/../../../etc/shadow"}, + wantError: fileutil.ErrInvalidJoinedPath, + }, + { + name: "path traversal to root", + entry: FileEntry{Base: backupData, Name: "../../../../../etc/crontab"}, + wantError: fileutil.ErrInvalidJoinedPath, + }, + { + name: "path traversal escapes ParentPath", + entry: FileEntry{Base: backupData, Name: "../../../../etc/passwd", ParentPath: "/tmp/restore"}, + wantError: fileutil.ErrInvalidJoinedPath, + }, + { + name: "relative path with dot-dot that stays within base", + entry: FileEntry{Base: backupData, Name: "mydb/../mydb/table1.ibd"}, + wantPath: "/vt/data/mydb/table1.ibd", + }, + } + + // Test unknown base separately since it returns a different error type. + t.Run("unknown base", func(t *testing.T) { + entry := FileEntry{Base: "unknown", Name: "file"} + _, err := entry.fullPath(cnf) + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown base") + }) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := tt.entry.fullPath(cnf) + if tt.wantError != nil { + require.ErrorIs(t, err, tt.wantError) + } else { + require.NoError(t, err) + assert.Equal(t, tt.wantPath, got) + } + }) + } +} + func TestShouldDrainForBackupBuiltIn(t *testing.T) { be := &BuiltinBackupEngine{}