Skip to content

Commit a7296fe

Browse files
rvrangeltimvaillancourt
rvrangel
and
timvaillancourt
authored
[slack-19.0]: backupengine: disallow path traversals via backup MANIFEST on restore (vitessio#19470) (#806)
* `backupengine`: disallow path traversals via backup `MANIFEST` on restore (vitessio#19470) Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * Address dir traversal in file backup storage `.ListBackups(...)` (vitessio#18814) Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> * fix for upstream cherry-pick --------- Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Tim Vaillancourt <tim@timvaillancourt.com>
1 parent 98ea8a3 commit a7296fe

5 files changed

Lines changed: 204 additions & 10 deletions

File tree

‎go/fileutil/join.go‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/*
2+
Copyright 2025 The Vitess Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package fileutil
18+
19+
import (
20+
"errors"
21+
"os"
22+
"path/filepath"
23+
"strings"
24+
)
25+
26+
var ErrInvalidJoinedPath = errors.New("invalid joined path")
27+
28+
// SafePathJoin joins file paths using a rootPath and one or many other paths,
29+
// returning a single absolute path. An error is returned if the joined path
30+
// causes a directory traversal to a path outside of the provided rootPath.
31+
func SafePathJoin(rootPath string, joinPaths ...string) (string, error) {
32+
allPaths := make([]string, 0, len(joinPaths)+1)
33+
allPaths = append(allPaths, rootPath)
34+
allPaths = append(allPaths, joinPaths...)
35+
p := filepath.Join(allPaths...)
36+
absPath, err := filepath.Abs(p)
37+
if err != nil {
38+
return p, err
39+
}
40+
absRootPath, err := filepath.Abs(rootPath)
41+
if err != nil {
42+
return absPath, err
43+
}
44+
if absPath != absRootPath && !strings.HasPrefix(absPath, absRootPath+string(os.PathSeparator)) {
45+
return absPath, ErrInvalidJoinedPath
46+
}
47+
return absPath, nil
48+
}

‎go/fileutil/join_test.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
Copyright 2025 The Vitess Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package fileutil
18+
19+
import (
20+
"path/filepath"
21+
"testing"
22+
23+
"github.com/stretchr/testify/require"
24+
)
25+
26+
func TestSafePathJoin(t *testing.T) {
27+
rootDir := t.TempDir()
28+
29+
t.Run("success", func(t *testing.T) {
30+
path, err := SafePathJoin(rootDir, "good/path")
31+
require.NoError(t, err)
32+
require.True(t, filepath.IsAbs(path))
33+
require.Equal(t, filepath.Join(rootDir, "good/path"), path)
34+
})
35+
36+
t.Run("dir-traversal", func(t *testing.T) {
37+
_, err := SafePathJoin(rootDir, "../../..")
38+
require.ErrorIs(t, err, ErrInvalidJoinedPath)
39+
})
40+
}

‎go/vt/mysqlctl/builtinbackupengine.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
"github.com/spf13/pflag"
3737
"golang.org/x/sync/semaphore"
3838

39+
"vitess.io/vitess/go/fileutil"
3940
"vitess.io/vitess/go/ioutil"
4041
"vitess.io/vitess/go/mysql"
4142
"vitess.io/vitess/go/mysql/replication"
@@ -167,7 +168,9 @@ func registerBuiltinBackupEngineFlags(fs *pflag.FlagSet) {
167168
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.")
168169
}
169170

170-
// fullPath returns the full path of the entry, based on its type
171+
// fullPath returns the full path of the entry, based on its type.
172+
// It validates that the resolved path does not escape the base directory
173+
// via path traversal (e.g. "../../" sequences in fe.Name).
171174
func (fe *FileEntry) fullPath(cnf *Mycnf) (string, error) {
172175
// find the root to use
173176
var root string
@@ -184,7 +187,7 @@ func (fe *FileEntry) fullPath(cnf *Mycnf) (string, error) {
184187
return "", vterrors.Errorf(vtrpc.Code_UNKNOWN, "unknown base: %v", fe.Base)
185188
}
186189

187-
return path.Join(fe.ParentPath, root, fe.Name), nil
190+
return fileutil.SafePathJoin(path.Join(fe.ParentPath, root), fe.Name)
188191
}
189192

190193
// open attempts t oopen the file

‎go/vt/mysqlctl/builtinbackupengine_test.go‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ import (
2121
"testing"
2222

2323
"github.com/stretchr/testify/assert"
24+
"github.com/stretchr/testify/require"
2425

26+
"vitess.io/vitess/go/fileutil"
2527
tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata"
2628
)
2729

@@ -70,6 +72,93 @@ func TestGetIncrementalFromPosGTIDSet(t *testing.T) {
7072
}
7173
}
7274

75+
func TestFileEntryFullPath(t *testing.T) {
76+
cnf := &Mycnf{
77+
DataDir: "/vt/data",
78+
InnodbDataHomeDir: "/vt/innodb-data",
79+
InnodbLogGroupHomeDir: "/vt/innodb-log",
80+
BinLogPath: "/vt/binlogs/mysql-bin",
81+
}
82+
83+
tests := []struct {
84+
name string
85+
entry FileEntry
86+
wantPath string
87+
wantError error
88+
}{
89+
{
90+
name: "valid relative path in DataDir",
91+
entry: FileEntry{Base: backupData, Name: "mydb/table1.ibd"},
92+
wantPath: "/vt/data/mydb/table1.ibd",
93+
},
94+
{
95+
name: "valid relative path in InnodbDataHomeDir",
96+
entry: FileEntry{Base: backupInnodbDataHomeDir, Name: "ibdata1"},
97+
wantPath: "/vt/innodb-data/ibdata1",
98+
},
99+
{
100+
name: "valid relative path in InnodbLogGroupHomeDir",
101+
entry: FileEntry{Base: backupInnodbLogGroupHomeDir, Name: "ib_logfile0"},
102+
wantPath: "/vt/innodb-log/ib_logfile0",
103+
},
104+
{
105+
name: "valid relative path in BinlogDir",
106+
entry: FileEntry{Base: backupBinlogDir, Name: "mysql-bin.000001"},
107+
wantPath: "/vt/binlogs/mysql-bin.000001",
108+
},
109+
{
110+
name: "valid path with ParentPath",
111+
entry: FileEntry{Base: backupData, Name: "mydb/table1.ibd", ParentPath: "/tmp/restore"},
112+
wantPath: "/tmp/restore/vt/data/mydb/table1.ibd",
113+
},
114+
{
115+
name: "path traversal escapes base directory",
116+
entry: FileEntry{Base: backupData, Name: "../../etc/passwd"},
117+
wantError: fileutil.ErrInvalidJoinedPath,
118+
},
119+
{
120+
name: "path traversal with deeper nesting",
121+
entry: FileEntry{Base: backupData, Name: "mydb/../../../etc/shadow"},
122+
wantError: fileutil.ErrInvalidJoinedPath,
123+
},
124+
{
125+
name: "path traversal to root",
126+
entry: FileEntry{Base: backupData, Name: "../../../../../etc/crontab"},
127+
wantError: fileutil.ErrInvalidJoinedPath,
128+
},
129+
{
130+
name: "path traversal escapes ParentPath",
131+
entry: FileEntry{Base: backupData, Name: "../../../../etc/passwd", ParentPath: "/tmp/restore"},
132+
wantError: fileutil.ErrInvalidJoinedPath,
133+
},
134+
{
135+
name: "relative path with dot-dot that stays within base",
136+
entry: FileEntry{Base: backupData, Name: "mydb/../mydb/table1.ibd"},
137+
wantPath: "/vt/data/mydb/table1.ibd",
138+
},
139+
}
140+
141+
// Test unknown base separately since it returns a different error type.
142+
t.Run("unknown base", func(t *testing.T) {
143+
entry := FileEntry{Base: "unknown", Name: "file"}
144+
_, err := entry.fullPath(cnf)
145+
require.Error(t, err)
146+
assert.Contains(t, err.Error(), "unknown base")
147+
})
148+
149+
for _, tt := range tests {
150+
t.Run(tt.name, func(t *testing.T) {
151+
got, err := tt.entry.fullPath(cnf)
152+
if tt.wantError != nil {
153+
require.ErrorIs(t, err, tt.wantError)
154+
} else {
155+
require.NoError(t, err)
156+
assert.Equal(t, tt.wantPath, got)
157+
}
158+
})
159+
}
160+
}
161+
73162
func TestShouldDrainForBackupBuiltIn(t *testing.T) {
74163
be := &BuiltinBackupEngine{}
75164

‎go/vt/mysqlctl/filebackupstorage/file.go‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727

2828
"github.com/spf13/pflag"
2929

30+
"vitess.io/vitess/go/fileutil"
3031
"vitess.io/vitess/go/ioutil"
3132
"vitess.io/vitess/go/vt/concurrency"
3233
stats "vitess.io/vitess/go/vt/mysqlctl/backupstats"
@@ -139,7 +140,10 @@ func (fbh *FileBackupHandle) ReadFile(ctx context.Context, filename string) (io.
139140
if !fbh.readOnly {
140141
return nil, fmt.Errorf("ReadFile cannot be called on read-write backup")
141142
}
142-
p := path.Join(FileBackupStorageRoot, fbh.dir, fbh.name, filename)
143+
p, err := fileutil.SafePathJoin(FileBackupStorageRoot, fbh.dir, fbh.name, filename)
144+
if err != nil {
145+
return nil, err
146+
}
143147
f, err := os.Open(p)
144148
if err != nil {
145149
return nil, err
@@ -159,9 +163,13 @@ func newFileBackupStorage(params backupstorage.Params) *FileBackupStorage {
159163

160164
// ListBackups is part of the BackupStorage interface
161165
func (fbs *FileBackupStorage) ListBackups(ctx context.Context, dir string) ([]backupstorage.BackupHandle, error) {
162-
// ReadDir already sorts the results
163-
p := path.Join(FileBackupStorageRoot, dir)
164-
fi, err := os.ReadDir(p)
166+
// Check dir is not a directory traversal.
167+
path, err := fileutil.SafePathJoin(FileBackupStorageRoot, dir)
168+
if err != nil {
169+
return nil, fmt.Errorf("failed to parse backup path %q: %w", path, err)
170+
}
171+
172+
fi, err := os.ReadDir(path)
165173
if err != nil {
166174
if os.IsNotExist(err) {
167175
return nil, nil
@@ -185,14 +193,17 @@ func (fbs *FileBackupStorage) ListBackups(ctx context.Context, dir string) ([]ba
185193
// StartBackup is part of the BackupStorage interface
186194
func (fbs *FileBackupStorage) StartBackup(ctx context.Context, dir, name string) (backupstorage.BackupHandle, error) {
187195
// Make sure the directory exists.
188-
p := path.Join(FileBackupStorageRoot, dir)
189-
if err := os.MkdirAll(p, os.ModePerm); err != nil {
196+
p, err := fileutil.SafePathJoin(FileBackupStorageRoot, dir)
197+
if err != nil {
198+
return nil, err
199+
}
200+
if err = os.MkdirAll(p, os.ModePerm); err != nil {
190201
return nil, err
191202
}
192203

193204
// Create the subdirectory for this named backup.
194205
p = path.Join(p, name)
195-
if err := os.Mkdir(p, os.ModePerm); err != nil {
206+
if err = os.Mkdir(p, os.ModePerm); err != nil {
196207
return nil, err
197208
}
198209

@@ -201,7 +212,10 @@ func (fbs *FileBackupStorage) StartBackup(ctx context.Context, dir, name string)
201212

202213
// RemoveBackup is part of the BackupStorage interface
203214
func (fbs *FileBackupStorage) RemoveBackup(ctx context.Context, dir, name string) error {
204-
p := path.Join(FileBackupStorageRoot, dir, name)
215+
p, err := fileutil.SafePathJoin(FileBackupStorageRoot, dir, name)
216+
if err != nil {
217+
return err
218+
}
205219
return os.RemoveAll(p)
206220
}
207221

0 commit comments

Comments
 (0)