CVE-2022-1992: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: Path Traversal in GitHub repository gogs/gogs prior to 0.12.9.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 9.1
Published: 2022-06-09T17:15Z
Repository: https://github.com/gogs/gogs
Language: Go
Files changed: CHANGELOG.md, internal/pathutil/pathutil.go, internal/pathutil/pathutil_test.go
Commit date: 2022-06-07 20:34:46 +0800
Commit message: pathutil: check both styles of `os.PathSeparator` (#7020)


Diff: diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9e932df2c3..765d69351d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,10 +23,11 @@ All notable changes to Gogs are documented in this file.
 
 ### Fixed
 
 - _Security:_ OS Command Injection in file editor. [#7000](https://github.com/gogs/gogs/issues/7000)
 - _Security:_ Sanitize `DisplayName` in repository issue list. [#7009](https://github.com/gogs/gogs/pull/7009)
+- _Security:_ Path Traversal in file editor on Windows. [#7001](https://github.com/gogs/gogs/issues/7001)
 - Unable to use LDAP authentication on ARM machines. [#6761](https://github.com/gogs/gogs/issues/6761)
 - Unable to init repository during creation on Windows. [#6967](https://github.com/gogs/gogs/issues/6967)
 - Mysterious panic on `Value not found for type *repo.HTTPContext`. [#6963](https://github.com/gogs/gogs/issues/6963)
 
 ### Removed
diff --git a/internal/pathutil/pathutil.go b/internal/pathutil/pathutil.go
index 6a7286e1c4..26ea76b6d5 100644
--- a/internal/pathutil/pathutil.go
+++ b/internal/pathutil/pathutil.go
@@ -7,9 +7,11 @@ package pathutil
 import (
 	"path"
 	"strings"
 )
 
-// Clean cleans up given path and returns a relative path that goes straight down.
+// Clean cleans up given path and returns a relative path that goes straight
+// down to prevent path traversal.
 func Clean(p string) string {
+	p = strings.ReplaceAll(p, `\`, "/")
 	return strings.Trim(path.Clean("/"+p), "/")
 }
diff --git a/internal/pathutil/pathutil_test.go b/internal/pathutil/pathutil_test.go
index eb8cc17630..d20e537a4f 100644
--- a/internal/pathutil/pathutil_test.go
+++ b/internal/pathutil/pathutil_test.go
@@ -10,40 +10,71 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
 func TestClean(t *testing.T) {
 	tests := []struct {
-		path   string
-		expVal string
+		path    string
+		wantVal string
 	}{
 		{
-			path:   "../../../readme.txt",
-			expVal: "readme.txt",
+			path:    "../../../readme.txt",
+			wantVal: "readme.txt",
 		},
 		{
-			path:   "a/../../../readme.txt",
-			expVal: "readme.txt",
+			path:    "a/../../../readme.txt",
+			wantVal: "readme.txt",
 		},
 		{
-			path:   "/../a/b/../c/../readme.txt",
-			expVal: "a/readme.txt",
+			path:    "/../a/b/../c/../readme.txt",
+			wantVal: "a/readme.txt",
 		},
 		{
-			path:   "/a/readme.txt",
-			expVal: "a/readme.txt",
+			path:    "/a/readme.txt",
+			wantVal: "a/readme.txt",
 		},
 		{
-			path:   "/",
-			expVal: "",
+			path:    "/",
+			wantVal: "",
 		},
 
 		{
-			path:   "/a/b/c/readme.txt",
-			expVal: "a/b/c/readme.txt",
+			path:    "/a/b/c/readme.txt",
+			wantVal: "a/b/c/readme.txt",
+		},
+
+		// Windows-specific
+		{
+			path:    `..\..\..\readme.txt`,
+			wantVal: "readme.txt",
+		},
+		{
+			path:    `a\..\..\..\readme.txt`,
+			wantVal: "readme.txt",
+		},
+		{
+			path:    `\..\a\b\..\c\..\readme.txt`,
+			wantVal: "a/readme.txt",
+		},
+		{
+			path:    `\a\readme.txt`,
+			wantVal: "a/readme.txt",
+		},
+		{
+			path:    `..\..\..\../README.md`,
+			wantVal: "README.md",
+		},
+		{
+			path:    `\`,
+			wantVal: "",
+		},
+
+		{
+			path:    `\a\b\c\readme.txt`,
+			wantVal: `a/b/c/readme.txt`,
 		},
 	}
 	for _, test := range tests {
-		t.Run("", func(t *testing.T) {
-			assert.Equal(t, test.expVal, Clean(test.path))
+		t.Run(test.path, func(t *testing.T) {
+			assert.Equal(t, test.wantVal, Clean(test.path))
 		})
 	}
 }

Vulnerable code: // Clean cleans up given path and returns a relative path that goes straight down.
		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",
		t.Run("", func(t *testing.T) {
			assert.Equal(t, test.expVal, Clean(test.path))
Fixed code: - _Security:_ Path Traversal in file editor on Windows. [#7001](https://github.com/gogs/gogs/issues/7001)
// Clean cleans up given path and returns a relative path that goes straight
// down to prevent path traversal.
	p = strings.ReplaceAll(p, `\`, "/")
		path    string
		wantVal string
			path:    "../../../readme.txt",
			wantVal: "readme.txt",
			path:    "a/../../../readme.txt",
			wantVal: "readme.txt",
			path:    "/../a/b/../c/../readme.txt",
			wantVal: "a/readme.txt",
			path:    "/a/readme.txt",
			wantVal: "a/readme.txt",
			path:    "/",
			wantVal: "",
			path:    "/a/b/c/readme.txt",
			wantVal: "a/b/c/readme.txt",
		},

		// Windows-specific
		{
			path:    `..\..\..\readme.txt`,
			wantVal: "readme.txt",
		},
		{
			path:    `a\..\..\..\readme.txt`,
			wantVal: "readme.txt",
		},
		{
			path:    `\..\a\b\..\c\..\readme.txt`,
			wantVal: "a/readme.txt",
		},
		{
			path:    `\a\readme.txt`,
			wantVal: "a/readme.txt",
		},
		{
			path:    `..\..\..\../README.md`,
			wantVal: "README.md",
		},
		{
			path:    `\`,
			wantVal: "",
		},

		{
			path:    `\a\b\c\readme.txt`,
			wantVal: `a/b/c/readme.txt`,
		t.Run(test.path, func(t *testing.T) {
			assert.Equal(t, test.wantVal, Clean(test.path))