Browse Source

repo/editor: clean up tree path

Fixes a security issue reported by @zeripath.
ᴜɴᴋɴᴡᴏɴ 4 years ago
parent
commit
ce1ec81d6f
3 changed files with 69 additions and 2 deletions
  1. 15 0
      internal/pathutil/pathutil.go
  2. 49 0
      internal/pathutil/pathutil_test.go
  3. 5 2
      internal/route/repo/editor.go

+ 15 - 0
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), "/")
+}

+ 49 - 0
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))
+		})
+	}
+}

+ 5 - 2
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.