Improve path component validation
Detect when a specific path component tries to escape the path that came
before it -- so that a user-provided value can't use '..' to escape the
directories laid out by the build system.
Change-Id: I02d52d9baadb7152448a34f4e8b573fe3c032b12
diff --git a/common/paths.go b/common/paths.go
index 2bbb72c..3ea9ef8 100644
--- a/common/paths.go
+++ b/common/paths.go
@@ -620,21 +620,24 @@
}
// validateSafePath validates a path that we trust (may contain ninja variables).
-// Ensures that it does not attempt to leave the containing directory.
+// Ensures that each path component does not attempt to leave its component.
func validateSafePath(ctx PathContext, paths ...string) string {
+ for _, path := range paths {
+ path := filepath.Clean(path)
+ if path == ".." || strings.HasPrefix(path, "../") || strings.HasPrefix(path, "/") {
+ reportPathError(ctx, "Path is outside directory: %s", path)
+ return ""
+ }
+ }
// TODO: filepath.Join isn't necessarily correct with embedded ninja
// variables. '..' may remove the entire ninja variable, even if it
// will be expanded to multiple nested directories.
- p := filepath.Join(paths...)
- if p == ".." || strings.HasPrefix(p, "../") || strings.HasPrefix(p, "/") {
- reportPathError(ctx, "Path is outside directory: %s", p)
- return ""
- }
- return p
+ return filepath.Join(paths...)
}
-// validatePath validates that a path does not include ninja variables, and does
-// not attempt to leave the containing directory.
+// validatePath validates that a path does not include ninja variables, and that
+// each path component does not attempt to leave its component. Returns a joined
+// version of each path component.
func validatePath(ctx PathContext, paths ...string) string {
for _, path := range paths {
if strings.Contains(path, "$") {
diff --git a/common/paths_test.go b/common/paths_test.go
index 16ede0d..c231163 100644
--- a/common/paths_test.go
+++ b/common/paths_test.go
@@ -69,6 +69,21 @@
out: "",
err: []error{errors.New("Path is outside directory: /a")},
},
+ {
+ in: []string{"a", "../b"},
+ out: "",
+ err: []error{errors.New("Path is outside directory: ../b")},
+ },
+ {
+ in: []string{"a", "b/../../c"},
+ out: "",
+ err: []error{errors.New("Path is outside directory: ../c")},
+ },
+ {
+ in: []string{"a", "./.."},
+ out: "",
+ err: []error{errors.New("Path is outside directory: ..")},
+ },
}
var validateSafePathTestCases = append(commonValidatePathTestCases, []strsTestCase{