From 3fafdb700197488667f52b01f2d08c62612e4bb5 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <jliggitt@redhat.com>
Date: Tue, 13 Mar 2018 00:59:40 -0400
Subject: [PATCH] Detect backsteps correctly in base path detection

Avoid false positives with atomic writer ..<timestamp> directories
---
 pkg/kubelet/util/util.go             | 13 -------------
 pkg/util/mount/mount.go              |  8 +++++++-
 pkg/util/mount/mount_linux_test.go   |  6 ++++++
 pkg/util/mount/mount_windows.go      |  4 ++--
 pkg/util/mount/mount_windows_test.go |  5 +++++
 5 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/pkg/kubelet/util/util.go b/pkg/kubelet/util/util.go
index cb70c07f86f..eb7cf142754 100644
--- a/pkg/kubelet/util/util.go
+++ b/pkg/kubelet/util/util.go
@@ -19,8 +19,6 @@ package util
 import (
 	"fmt"
 	"net/url"
-	"path/filepath"
-	"strings"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -47,14 +45,3 @@ func parseEndpoint(endpoint string) (string, string, error) {
 		return u.Scheme, "", fmt.Errorf("protocol %q not supported", u.Scheme)
 	}
 }
-
-func pathWithinBase(fullPath, basePath string) bool {
-	rel, err := filepath.Rel(basePath, fullPath)
-	if err != nil {
-		return false
-	}
-	if strings.HasPrefix(rel, "..") {
-		return false
-	}
-	return true
-}
diff --git a/pkg/util/mount/mount.go b/pkg/util/mount/mount.go
index f241679b565..f30db7692e8 100644
--- a/pkg/util/mount/mount.go
+++ b/pkg/util/mount/mount.go
@@ -325,9 +325,15 @@ func pathWithinBase(fullPath, basePath string) bool {
 	if err != nil {
 		return false
 	}
-	if strings.HasPrefix(rel, "..") {
+	if startsWithBackstep(rel) {
 		// Needed to escape the base path
 		return false
 	}
 	return true
 }
+
+// startsWithBackstep checks if the given path starts with a backstep segment
+func startsWithBackstep(rel string) bool {
+	// normalize to / and check for ../
+	return rel == ".." || strings.HasPrefix(filepath.ToSlash(rel), "../")
+}
diff --git a/pkg/util/mount/mount_linux_test.go b/pkg/util/mount/mount_linux_test.go
index 25bdfc6fe6f..14e0c9af49e 100644
--- a/pkg/util/mount/mount_linux_test.go
+++ b/pkg/util/mount/mount_linux_test.go
@@ -402,6 +402,12 @@ func TestPathWithinBase(t *testing.T) {
 			basePath: "/a",
 			expected: false,
 		},
+		{
+			name:     "configmap subpath",
+			fullPath: "/var/lib/kubelet/pods/uuid/volumes/kubernetes.io~configmap/config/..timestamp/file.txt",
+			basePath: "/var/lib/kubelet/pods/uuid/volumes/kubernetes.io~configmap/config",
+			expected: true,
+		},
 	}
 	for _, test := range tests {
 		if pathWithinBase(test.fullPath, test.basePath) != test.expected {
diff --git a/pkg/util/mount/mount_windows.go b/pkg/util/mount/mount_windows.go
index 9ba4417f979..12e1bca118a 100644
--- a/pkg/util/mount/mount_windows.go
+++ b/pkg/util/mount/mount_windows.go
@@ -291,7 +291,7 @@ func lockAndCheckSubPathWithoutSymlink(volumePath, subPath string) ([]uintptr, e
 	if err != nil {
 		return []uintptr{}, fmt.Errorf("Rel(%s, %s) error: %v", volumePath, subPath, err)
 	}
-	if strings.HasPrefix(relSubPath, "..") {
+	if startsWithBackstep(relSubPath) {
 		return []uintptr{}, fmt.Errorf("SubPath %q not within volume path %q", subPath, volumePath)
 	}
 
@@ -552,7 +552,7 @@ func findExistingPrefix(base, pathname string) (string, []string, error) {
 		return base, nil, err
 	}
 
-	if strings.HasPrefix(rel, "..") {
+	if startsWithBackstep(rel) {
 		return base, nil, fmt.Errorf("pathname(%s) is not within base(%s)", pathname, base)
 	}
 
diff --git a/pkg/util/mount/mount_windows_test.go b/pkg/util/mount/mount_windows_test.go
index 76ef08ccc9d..e31217e975d 100644
--- a/pkg/util/mount/mount_windows_test.go
+++ b/pkg/util/mount/mount_windows_test.go
@@ -538,6 +538,11 @@ func TestPathWithinBase(t *testing.T) {
 			basePath:       `c:\tmp\a\b\c`,
 			expectedResult: false,
 		},
+		{
+			fullPath:       `c:\kubelet\pods\uuid\volumes\kubernetes.io~configmap\config\..timestamp\file.txt`,
+			basePath:       `c:\kubelet\pods\uuid\volumes\kubernetes.io~configmap\config`,
+			expectedResult: true,
+		},
 	}
 
 	for _, test := range tests {