Extend safeTemplateFilepath to cover more cases.

CHANGELOG.md

@@ -76,6 +76,7 @@
 * [BUGFIX] Multikv: Fix watching for runtime config changes in `multi` KV store in ruler and querier. #1665
 * [BUGFIX] Memcached: allow to use CNAME DNS records for the memcached backend addresses. #1654
 * [BUGFIX] Querier: fixed temporary partial query results when shuffle sharding is enabled and hash ring backend storage is flushed / reset. #1829
+* [BUGFIX] Alertmanager: prevent more file traversal cases related to template names. #1833
 
 ### Mixin
 

pkg/alertmanager/multitenant.go

@@ -1262,16 +1262,42 @@ func safeTemplateFilepath(dir, templateName string) (string, error) {
 		return "", err
 	}
 
+	// If actualPath is same as containerDir, it's likely that actualPath was empty, or just ".".
+	if containerDir == actualPath {
+		return "", fmt.Errorf("invalid template name %q", templateName)
+	}
+
 	// Ensure the actual path of the template is within the expected directory.
 	// This check is a counter-measure to make sure the tenant is not trying to
 	// escape its own directory on disk.
-	if !strings.HasPrefix(actualPath, containerDir) {
+	if !isFilePathInsideDirectory(containerDir, actualPath) {
 		return "", fmt.Errorf("invalid template name %q: the template filepath is escaping the per-tenant local directory", templateName)
 	}
 
 	return actualPath, nil
 }
 
+// Returns true if file described by filePath is inside given directory. (Doesn't check for symbolic links)
+// Directory argument should not end with separator, ie. it must be filepath.Clean-ed.
+func isFilePathInsideDirectory(absDir string, absFilePath string) bool {
+	if absFilePath == absDir {
+		// If filePath refers to dir itself, it's not a "file" path.
+		return false
+	}
+
+	d := filepath.Dir(absFilePath)
+	if d == absDir {
+		return true
+	}
+
+	// If path was empty, its Dir is ".".
+	// If path was "/", its Dir is just "/".
+	if d == "." || d == "/" {
+		return false
+	}
+	return isFilePathInsideDirectory(absDir, d)
+}
+
 // storeTemplateFile stores template file at the given templateFilepath.
 // Returns true, if file content has changed (new or updated file), false if file with the same name
 // and content was already stored locally.

pkg/alertmanager/multitenant_test.go

@@ -1969,6 +1969,27 @@ func TestSafeTemplateFilepath(t *testing.T) {
 			template:    "../test.tmpl",
 			expectedErr: errors.New(`invalid template name "../test.tmpl": the template filepath is escaping the per-tenant local directory`),
 		},
+		"template name starting with /": {
+			dir:          "/tmp",
+			template:     "/file",
+			expectedErr:  nil,
+			expectedPath: "/tmp/file",
+		},
+		"escaping template name that has prefix of dir (tmp is prefix of tmpfile)": {
+			dir:         "/sub/tmp",
+			template:    "../tmpfile",
+			expectedErr: errors.New(`invalid template name "../tmpfile": the template filepath is escaping the per-tenant local directory`),
+		},
+		"empty template name": {
+			dir:         "/tmp",
+			template:    "",
+			expectedErr: errors.New(`invalid template name ""`),
+		},
+		"dot template name": {
+			dir:         "/tmp",
+			template:    ".",
+			expectedErr: errors.New(`invalid template name "."`),
+		},
 	}
 
 	for testName, testData := range tests {
@@ -1980,6 +2001,27 @@ func TestSafeTemplateFilepath(t *testing.T) {
 	}
 }
 
+func TestIsFilePathInsideDirectory(t *testing.T) {
+	assert.False(t, isFilePathInsideDirectory("/", "/"))
+	assert.False(t, isFilePathInsideDirectory("/", ""))
+	assert.True(t, isFilePathInsideDirectory("/", "/test"))
+	assert.False(t, isFilePathInsideDirectory("/", "random"))
+
+	assert.False(t, isFilePathInsideDirectory("/tmp", "/"))
+	assert.False(t, isFilePathInsideDirectory("/tmp", "//"))
+	assert.False(t, isFilePathInsideDirectory("/tmp", ""))
+	assert.False(t, isFilePathInsideDirectory("/tmp", "/tmp"))
+	assert.False(t, isFilePathInsideDirectory("/tmp", "/tmpfile"))
+	assert.True(t, isFilePathInsideDirectory("/tmp", "/tmp/test"))
+	assert.True(t, isFilePathInsideDirectory("/tmp", "/tmp/inner/dir/file"))
+	// Dir ending with / doesn't work.
+	assert.False(t, isFilePathInsideDirectory("/tmp/", "/tmp/inner/dir/file"))
+
+	assert.True(t, isFilePathInsideDirectory(".", "./test"))
+	// Dir ending with / doesn't work.
+	assert.False(t, isFilePathInsideDirectory("./", "./test"))
+}
+
 func TestStoreTemplateFile(t *testing.T) {
 	tempDir := t.TempDir()
 	testTemplateDir := filepath.Join(tempDir, templatesDir)