From 92e85623a0cd7b2043a79b757e1874a67796dea9 Mon Sep 17 00:00:00 2001
From: Nicolas Lacasse <nlacasse@google.com>
Date: Fri, 1 Feb 2019 15:22:22 -0800
Subject: Factor the subtargets method into a helper method with tests.

PiperOrigin-RevId: 232047515
Change-Id: I00f036816e320356219be7b2f2e6d5fe57583a60
---
 pkg/sentry/fs/path.go      | 27 ++++++++++++++++
 pkg/sentry/fs/path_test.go | 78 ++++++++++++++++++++++++++++++++++++++++++++++
 runsc/boot/fs.go           | 14 ++-------
 3 files changed, 107 insertions(+), 12 deletions(-)

diff --git a/pkg/sentry/fs/path.go b/pkg/sentry/fs/path.go
index 91a9a8ffd..52139b648 100644
--- a/pkg/sentry/fs/path.go
+++ b/pkg/sentry/fs/path.go
@@ -14,6 +14,11 @@
 
 package fs
 
+import (
+	"path/filepath"
+	"strings"
+)
+
 // TrimTrailingSlashes trims any trailing slashes.
 //
 // The returned boolean indicates whether any changes were made.
@@ -90,3 +95,25 @@ func SplitFirst(path string) (current, remainder string) {
 		return current, remainder
 	}
 }
+
+// IsSubpath checks whether the first path is a (strict) descendent of the
+// second. If it is a subpath, then true is returned along with a clean
+// relative path from the second path to the first. Otherwise false is
+// returned.
+func IsSubpath(subpath, path string) (string, bool) {
+	cleanPath := filepath.Clean(path)
+	cleanSubpath := filepath.Clean(subpath)
+
+	// Add a trailing slash to the path if it does not already have one.
+	if len(cleanPath) == 0 || cleanPath[len(cleanPath)-1] != '/' {
+		cleanPath += "/"
+	}
+	if cleanPath == cleanSubpath {
+		// Paths are equal, thus not a strict subpath.
+		return "", false
+	}
+	if strings.HasPrefix(cleanSubpath, cleanPath) {
+		return strings.TrimPrefix(cleanSubpath, cleanPath), true
+	}
+	return "", false
+}
diff --git a/pkg/sentry/fs/path_test.go b/pkg/sentry/fs/path_test.go
index 391b010a7..4ba1498f6 100644
--- a/pkg/sentry/fs/path_test.go
+++ b/pkg/sentry/fs/path_test.go
@@ -209,3 +209,81 @@ func TestSplitFirst(t *testing.T) {
 		}
 	}
 }
+
+// TestIsSubpath tests the IsSubpath method.
+func TestIsSubpath(t *testing.T) {
+	tcs := []struct {
+		// Two absolute paths.
+		pathA string
+		pathB string
+
+		// Whether pathA is a subpath of pathB.
+		wantIsSubpath bool
+
+		// Relative path from pathA to pathB. Only checked if
+		// wantIsSubpath is true.
+		wantRelpath string
+	}{
+		{
+			pathA:         "/foo/bar/baz",
+			pathB:         "/foo",
+			wantIsSubpath: true,
+			wantRelpath:   "bar/baz",
+		},
+		{
+			pathA:         "/foo",
+			pathB:         "/foo/bar/baz",
+			wantIsSubpath: false,
+		},
+		{
+			pathA:         "/foo",
+			pathB:         "/foo",
+			wantIsSubpath: false,
+		},
+		{
+			pathA:         "/foobar",
+			pathB:         "/foo",
+			wantIsSubpath: false,
+		},
+		{
+			pathA:         "/foo",
+			pathB:         "/foobar",
+			wantIsSubpath: false,
+		},
+		{
+			pathA:         "/foo",
+			pathB:         "/foobar",
+			wantIsSubpath: false,
+		},
+		{
+			pathA:         "/",
+			pathB:         "/foo",
+			wantIsSubpath: false,
+		},
+		{
+			pathA:         "/foo",
+			pathB:         "/",
+			wantIsSubpath: true,
+			wantRelpath:   "foo",
+		},
+		{
+			pathA:         "/foo/bar/../bar",
+			pathB:         "/foo",
+			wantIsSubpath: true,
+			wantRelpath:   "bar",
+		},
+		{
+			pathA:         "/foo/bar",
+			pathB:         "/foo/../foo",
+			wantIsSubpath: true,
+			wantRelpath:   "bar",
+		},
+	}
+
+	for _, tc := range tcs {
+		gotRelpath, gotIsSubpath := IsSubpath(tc.pathA, tc.pathB)
+		if gotRelpath != tc.wantRelpath || gotIsSubpath != tc.wantIsSubpath {
+			t.Errorf("IsSubpath(%q, %q) got %q %t, want %q %t", tc.pathA, tc.pathB, gotRelpath, gotIsSubpath, tc.wantRelpath, tc.wantIsSubpath)
+		}
+	}
+}
diff --git a/runsc/boot/fs.go b/runsc/boot/fs.go
index 5c5e650ca..ada292c9e 100644
--- a/runsc/boot/fs.go
+++ b/runsc/boot/fs.go
@@ -515,20 +515,10 @@ func addSubmountOverlay(ctx context.Context, inode *fs.Inode, submounts []string
 // subtargets takes a set of Mounts and returns only the targets that are
 // children of the given root. The returned paths are relative to the root.
 func subtargets(root string, mnts []specs.Mount) []string {
-	r := filepath.Clean(root)
-	if len(r) > 0 && r[len(r)-1] != '/' {
-		r += "/"
-	}
 	var targets []string
 	for _, mnt := range mnts {
-		t := filepath.Clean(mnt.Destination)
-		if strings.HasPrefix(t, r) {
-			// Make the mnt path relative to the root path.  If the
-			// result is empty, then mnt IS the root mount, not a
-			// submount.  We don't want to include those.
-			if t := strings.TrimPrefix(t, r); t != "" {
-				targets = append(targets, t)
-			}
+		if relPath, isSubpath := fs.IsSubpath(mnt.Destination, root); isSubpath {
+			targets = append(targets, relPath)
 		}
 	}
 	return targets
-- 
cgit v1.2.3