backupengine: disallow path traversals via backup MANIFEST on restore

go/vt/mysqlctl/builtinbackupengine.go

@@ -30,6 +30,7 @@ import (
 	"path"
 	"path/filepath"
 	"strconv"
+	"strings"
 	"sync/atomic"
 	"time"
 
@@ -180,7 +181,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
@@ -197,7 +200,14 @@ func (fe *FileEntry) fullPath(cnf *Mycnf) (string, error) {
 		return "", vterrors.Errorf(vtrpcpb.Code_UNKNOWN, "unknown base: %v", fe.Base)
 	}
 
-	return path.Join(fe.ParentPath, root, fe.Name), nil
+	baseDir := filepath.Clean(path.Join(fe.ParentPath, root))
+	resolved := filepath.Clean(path.Join(baseDir, fe.Name))
+
+	if !strings.HasPrefix(resolved, baseDir+string(filepath.Separator)) && resolved != baseDir {
+		return "", vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "path traversal not allowed: name %q escapes base directory %q", fe.Name, baseDir)
+	}
+
+	return resolved, nil
 }
 
 // open attempts to open the file

go/vt/mysqlctl/builtinbackupengine_test.go

@@ -21,6 +21,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata"
 )
@@ -70,6 +71,91 @@ 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 string
+	}{
+		{
+			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: "path traversal not allowed",
+		},
+		{
+			name:      "path traversal with deeper nesting",
+			entry:     FileEntry{Base: backupData, Name: "mydb/../../../etc/shadow"},
+			wantError: "path traversal not allowed",
+		},
+		{
+			name:      "path traversal to root",
+			entry:     FileEntry{Base: backupData, Name: "../../../../../etc/crontab"},
+			wantError: "path traversal not allowed",
+		},
+		{
+			name:      "path traversal escapes ParentPath",
+			entry:     FileEntry{Base: backupData, Name: "../../../../etc/passwd", ParentPath: "/tmp/restore"},
+			wantError: "path traversal not allowed",
+		},
+		{
+			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",
+		},
+		{
+			name:      "unknown base",
+			entry:     FileEntry{Base: "unknown", Name: "file"},
+			wantError: "unknown base",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := tt.entry.fullPath(cnf)
+			if tt.wantError != "" {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), tt.wantError)
+			} else {
+				require.NoError(t, err)
+				assert.Equal(t, tt.wantPath, got)
+			}
+		})
+	}
+}
+
 func TestShouldDrainForBackupBuiltIn(t *testing.T) {
 	be := &BuiltinBackupEngine{}