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

2 files changed, 27 insertions, 9 deletions

diff --git a/common/paths.go b/common/paths.go
index 2bbb72c4..3ea9ef80 100644
--- a/common/paths.go
+++ b/common/paths.go
@@ -620,21 +620,24 @@ func PathForModuleInstall(ctx AndroidModuleContext, paths ...string) OutputPath
 }
 
 // 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 16ede0d2..c2311630 100644
--- a/common/paths_test.go
+++ b/common/paths_test.go
@@ -69,6 +69,21 @@ var commonValidatePathTestCases = []strsTestCase{
 		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{