From 9a9388ace25bd646f5098cb9193d983332c34e41 Mon Sep 17 00:00:00 2001
From: Joe Chen <jc@unknwon.io>
Date: Sat, 14 Dec 2024 21:30:34 -0500
Subject: [PATCH] api: clean file path for updating repo contents (#7859)

## Describe the pull request

Link to the issue: closes https://github.com/gogs/gogs/issues/7582
---
 internal/database/repo_editor.go       | 1 +
 internal/pathutil/pathutil.go          | 3 +++
 internal/route/api/v1/repo/contents.go | 9 +++++++--
 internal/route/repo/editor.go          | 3 +++
 internal/route/repo/http.go            | 1 +
 5 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/internal/database/repo_editor.go b/internal/database/repo_editor.go
index f1b7a245b..c0be2528d 100644
--- a/internal/database/repo_editor.go
+++ b/internal/database/repo_editor.go
@@ -541,6 +541,7 @@ func (repo *Repository) UploadRepoFiles(doer *User, opts UploadRepoFileOptions)
 			continue
 		}
 
+		// 🚨 SECURITY: Prevent path traversal.
 		upload.Name = pathutil.Clean(upload.Name)
 
 		// 🚨 SECURITY: Prevent uploading files into the ".git" directory
diff --git a/internal/pathutil/pathutil.go b/internal/pathutil/pathutil.go
index 26ea76b6d..c2e53fdbc 100644
--- a/internal/pathutil/pathutil.go
+++ b/internal/pathutil/pathutil.go
@@ -11,6 +11,9 @@ import (
 
 // Clean cleans up given path and returns a relative path that goes straight
 // down to prevent path traversal.
+//
+// 🚨 SECURITY: This function MUST be used for any user input that is used as
+// file system path to prevent path traversal.
 func Clean(p string) string {
 	p = strings.ReplaceAll(p, `\`, "/")
 	return strings.Trim(path.Clean("/"+p), "/")
diff --git a/internal/route/api/v1/repo/contents.go b/internal/route/api/v1/repo/contents.go
index e7c36c75e..7d2d6fbe9 100644
--- a/internal/route/api/v1/repo/contents.go
+++ b/internal/route/api/v1/repo/contents.go
@@ -16,6 +16,7 @@ import (
 	"gogs.io/gogs/internal/context"
 	"gogs.io/gogs/internal/database"
 	"gogs.io/gogs/internal/gitutil"
+	"gogs.io/gogs/internal/pathutil"
 	"gogs.io/gogs/internal/repoutil"
 )
 
@@ -120,7 +121,8 @@ func GetContents(c *context.APIContext) {
 		return
 	}
 
-	treePath := c.Params("*")
+	// 🚨 SECURITY: Prevent path traversal.
+	treePath := pathutil.Clean(c.Params("*"))
 	entry, err := commit.TreeEntry(treePath)
 	if err != nil {
 		c.NotFoundOrError(gitutil.NewError(err), "get tree entry")
@@ -188,7 +190,10 @@ func PutContents(c *context.APIContext, r PutContentsRequest) {
 	if r.Branch == "" {
 		r.Branch = c.Repo.Repository.DefaultBranch
 	}
-	treePath := c.Params("*")
+
+	// 🚨 SECURITY: Prevent path traversal.
+	treePath := pathutil.Clean(c.Params("*"))
+
 	err = c.Repo.Repository.UpdateRepoFile(
 		c.User,
 		database.UpdateRepoFileOptions{
diff --git a/internal/route/repo/editor.go b/internal/route/repo/editor.go
index 90b1d0533..c71ac165f 100644
--- a/internal/route/repo/editor.go
+++ b/internal/route/repo/editor.go
@@ -135,6 +135,7 @@ func editFilePost(c *context.Context, f form.EditRepoFile, isNewFile bool) {
 		branchName = f.NewBranchName
 	}
 
+	// 🚨 SECURITY: Prevent path traversal.
 	f.TreePath = pathutil.Clean(f.TreePath)
 	treeNames, treePaths := getParentTreeFields(f.TreePath)
 
@@ -342,6 +343,7 @@ func DeleteFilePost(c *context.Context, f form.DeleteRepoFile) {
 	c.PageIs("Delete")
 	c.Data["BranchLink"] = c.Repo.RepoLink + "/src/" + c.Repo.BranchName
 
+	// 🚨 SECURITY: Prevent path traversal.
 	c.Repo.TreePath = pathutil.Clean(c.Repo.TreePath)
 	c.Data["TreePath"] = c.Repo.TreePath
 
@@ -437,6 +439,7 @@ func UploadFilePost(c *context.Context, f form.UploadRepoFile) {
 		branchName = f.NewBranchName
 	}
 
+	// 🚨 SECURITY: Prevent path traversal.
 	f.TreePath = pathutil.Clean(f.TreePath)
 	treeNames, treePaths := getParentTreeFields(f.TreePath)
 	if len(treeNames) == 0 {
diff --git a/internal/route/repo/http.go b/internal/route/repo/http.go
index 520ea8180..dbdd7f10a 100644
--- a/internal/route/repo/http.go
+++ b/internal/route/repo/http.go
@@ -411,6 +411,7 @@ func HTTP(c *HTTPContext) {
 			return
 		}
 
+		// 🚨 SECURITY: Prevent path traversal.
 		cleaned := pathutil.Clean(m[1])
 		if m[1] != "/"+cleaned {
 			c.Error(http.StatusBadRequest, "Request path contains suspicious characters")