From ce1ec81d6fa4b9531eee8c51a6ce1a095ea9beb2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E1=B4=9C=C9=B4=E1=B4=8B=C9=B4=E1=B4=A1=E1=B4=8F=C9=B4?=
 <u@gogs.io>
Date: Wed, 19 Feb 2020 23:45:02 +0800
Subject: [PATCH] repo/editor: clean up tree path

Fixes a security issue reported by @zeripath.
---
 internal/pathutil/pathutil.go      | 15 +++++++++
 internal/pathutil/pathutil_test.go | 49 ++++++++++++++++++++++++++++++
 internal/route/repo/editor.go      |  7 +++--
 3 files changed, 69 insertions(+), 2 deletions(-)
 create mode 100644 internal/pathutil/pathutil.go
 create mode 100644 internal/pathutil/pathutil_test.go

diff --git a/internal/pathutil/pathutil.go b/internal/pathutil/pathutil.go
new file mode 100644
index 000000000..6a7286e1c
--- /dev/null
+++ b/internal/pathutil/pathutil.go
@@ -0,0 +1,15 @@
+// Copyright 2020 The Gogs Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package pathutil
+
+import (
+	"path"
+	"strings"
+)
+
+// Clean cleans up given path and returns a relative path that goes straight down.
+func Clean(p string) string {
+	return strings.Trim(path.Clean("/"+p), "/")
+}
diff --git a/internal/pathutil/pathutil_test.go b/internal/pathutil/pathutil_test.go
new file mode 100644
index 000000000..eb8cc1763
--- /dev/null
+++ b/internal/pathutil/pathutil_test.go
@@ -0,0 +1,49 @@
+// Copyright 2020 The Gogs Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package pathutil
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestClean(t *testing.T) {
+	tests := []struct {
+		path   string
+		expVal string
+	}{
+		{
+			path:   "../../../readme.txt",
+			expVal: "readme.txt",
+		},
+		{
+			path:   "a/../../../readme.txt",
+			expVal: "readme.txt",
+		},
+		{
+			path:   "/../a/b/../c/../readme.txt",
+			expVal: "a/readme.txt",
+		},
+		{
+			path:   "/a/readme.txt",
+			expVal: "a/readme.txt",
+		},
+		{
+			path:   "/",
+			expVal: "",
+		},
+
+		{
+			path:   "/a/b/c/readme.txt",
+			expVal: "a/b/c/readme.txt",
+		},
+	}
+	for _, test := range tests {
+		t.Run("", func(t *testing.T) {
+			assert.Equal(t, test.expVal, Clean(test.path))
+		})
+	}
+}
diff --git a/internal/route/repo/editor.go b/internal/route/repo/editor.go
index a3ca3d704..54f6c20be 100644
--- a/internal/route/repo/editor.go
+++ b/internal/route/repo/editor.go
@@ -18,6 +18,7 @@ import (
 	"gogs.io/gogs/internal/db"
 	"gogs.io/gogs/internal/db/errors"
 	"gogs.io/gogs/internal/form"
+	"gogs.io/gogs/internal/pathutil"
 	"gogs.io/gogs/internal/setting"
 	"gogs.io/gogs/internal/template"
 	"gogs.io/gogs/internal/tool"
@@ -141,7 +142,7 @@ func editFilePost(c *context.Context, f form.EditRepoFile, isNewFile bool) {
 		branchName = f.NewBranchName
 	}
 
-	f.TreePath = strings.Trim(path.Clean("/"+f.TreePath), " /")
+	f.TreePath = pathutil.Clean(f.TreePath)
 	treeNames, treePaths := getParentTreeFields(f.TreePath)
 
 	c.Data["ParentTreePath"] = path.Dir(c.Repo.TreePath)
@@ -339,6 +340,8 @@ func DeleteFile(c *context.Context) {
 func DeleteFilePost(c *context.Context, f form.DeleteRepoFile) {
 	c.PageIs("Delete")
 	c.Data["BranchLink"] = c.Repo.RepoLink + "/src/" + c.Repo.BranchName
+
+	c.Repo.TreePath = pathutil.Clean(c.Repo.TreePath)
 	c.Data["TreePath"] = c.Repo.TreePath
 
 	oldBranchName := c.Repo.BranchName
@@ -433,7 +436,7 @@ func UploadFilePost(c *context.Context, f form.UploadRepoFile) {
 		branchName = f.NewBranchName
 	}
 
-	f.TreePath = strings.Trim(path.Clean("/"+f.TreePath), " /")
+	f.TreePath = pathutil.Clean(f.TreePath)
 	treeNames, treePaths := getParentTreeFields(f.TreePath)
 	if len(treeNames) == 0 {
 		// We must at least have one element for user to input.