From 80f4cea1ad15b1d7cec3d2e20e92f7aef08dde8b Mon Sep 17 00:00:00 2001
From: Paul Duffin <paulduffin@google.com>
Date: Tue, 16 Mar 2021 14:08:00 +0000
Subject: [PATCH] Prevent invalid paths being added to mock file system

Bug: 182885307
Test: m nothing
Change-Id: Ie9f60fd02e2a2bc44811dbcadf0eada4e52c9749
---
 android/fixture.go      | 29 +++++++++++++++++++++++++++++
 android/fixture_test.go | 39 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/android/fixture.go b/android/fixture.go
index 4e445c0b0..a37253442 100644
--- a/android/fixture.go
+++ b/android/fixture.go
@@ -16,6 +16,7 @@ package android
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 )
 
@@ -241,6 +242,7 @@ type MockFS map[string][]byte
 // Fails if the supplied map files with the same paths are present in both of them.
 func (fs MockFS) Merge(extra map[string][]byte) {
 	for p, c := range extra {
+		validateFixtureMockFSPath(p)
 		if _, ok := fs[p]; ok {
 			panic(fmt.Errorf("attempted to add file %s to the mock filesystem but it already exists", p))
 		}
@@ -248,6 +250,27 @@ func (fs MockFS) Merge(extra map[string][]byte) {
 	}
 }
 
+// Ensure that tests cannot add paths into the mock file system which would not be allowed in the
+// runtime, e.g. absolute paths, paths relative to the 'out/' directory.
+func validateFixtureMockFSPath(path string) {
+	// This uses validateSafePath rather than validatePath because the latter prevents adding files
+	// that include a $ but there are tests that allow files with a $ to be used, albeit only by
+	// globbing.
+	validatedPath, err := validateSafePath(path)
+	if err != nil {
+		panic(err)
+	}
+
+	// Make sure that the path is canonical.
+	if validatedPath != path {
+		panic(fmt.Errorf("path %q is not a canonical path, use %q instead", path, validatedPath))
+	}
+
+	if path == "out" || strings.HasPrefix(path, "out/") {
+		panic(fmt.Errorf("cannot add output path %q to the mock file system", path))
+	}
+}
+
 func (fs MockFS) AddToFixture() FixturePreparer {
 	return FixtureMergeMockFs(fs)
 }
@@ -281,6 +304,11 @@ func FixtureRegisterWithContext(registeringFunc func(ctx RegistrationContext)) F
 func FixtureModifyMockFS(mutator func(fs MockFS)) FixturePreparer {
 	return newSimpleFixturePreparer(func(f *fixture) {
 		mutator(f.mockFS)
+
+		// Make sure that invalid paths were not added to the mock filesystem.
+		for p, _ := range f.mockFS {
+			validateFixtureMockFSPath(p)
+		}
 	})
 }
 
@@ -298,6 +326,7 @@ func FixtureMergeMockFs(mockFS MockFS) FixturePreparer {
 // Fail if the filesystem already contains a file with that path, use FixtureOverrideFile instead.
 func FixtureAddFile(path string, contents []byte) FixturePreparer {
 	return FixtureModifyMockFS(func(fs MockFS) {
+		validateFixtureMockFSPath(path)
 		if _, ok := fs[path]; ok {
 			panic(fmt.Errorf("attempted to add file %s to the mock filesystem but it already exists, use FixtureOverride*File instead", path))
 		}
diff --git a/android/fixture_test.go b/android/fixture_test.go
index 0042e5b16..209bee680 100644
--- a/android/fixture_test.go
+++ b/android/fixture_test.go
@@ -14,7 +14,9 @@
 
 package android
 
-import "testing"
+import (
+	"testing"
+)
 
 // Make sure that FixturePreparer instances are only called once per fixture and in the order in
 // which they were added.
@@ -47,3 +49,38 @@ func TestFixtureDedup(t *testing.T) {
 	AssertDeepEquals(t, "preparers called in wrong order",
 		[]string{"preparer1", "preparer2", "preparer4", "preparer3"}, list)
 }
+
+func TestFixtureValidateMockFS(t *testing.T) {
+	buildDir := "<unused>"
+	factory := NewFixtureFactory(&buildDir)
+
+	t.Run("absolute path", func(t *testing.T) {
+		AssertPanicMessageContains(t, "source path validation failed", "Path is outside directory: /abs/path/Android.bp", func() {
+			factory.Fixture(t, FixtureAddFile("/abs/path/Android.bp", nil))
+		})
+	})
+	t.Run("not canonical", func(t *testing.T) {
+		AssertPanicMessageContains(t, "source path validation failed", `path "path/with/../in/it/Android.bp" is not a canonical path, use "path/in/it/Android.bp" instead`, func() {
+			factory.Fixture(t, FixtureAddFile("path/with/../in/it/Android.bp", nil))
+		})
+	})
+	t.Run("FixtureAddFile", func(t *testing.T) {
+		AssertPanicMessageContains(t, "source path validation failed", `cannot add output path "out/Android.bp" to the mock file system`, func() {
+			factory.Fixture(t, FixtureAddFile("out/Android.bp", nil))
+		})
+	})
+	t.Run("FixtureMergeMockFs", func(t *testing.T) {
+		AssertPanicMessageContains(t, "source path validation failed", `cannot add output path "out/Android.bp" to the mock file system`, func() {
+			factory.Fixture(t, FixtureMergeMockFs(MockFS{
+				"out/Android.bp": nil,
+			}))
+		})
+	})
+	t.Run("FixtureModifyMockFS", func(t *testing.T) {
+		AssertPanicMessageContains(t, "source path validation failed", `cannot add output path "out/Android.bp" to the mock file system`, func() {
+			factory.Fixture(t, FixtureModifyMockFS(func(fs MockFS) {
+				fs["out/Android.bp"] = nil
+			}))
+		})
+	})
+}