CVE-2022-1986: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: OS Command Injection in GitHub repository gogs/gogs prior to 0.12.9.
CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
CVSS v3 base score: 9.8
Published: 2022-06-09T17:15Z
Repository: https://github.com/gogs/gogs
Language: Go
Files changed: CHANGELOG.md, internal/db/repo_editor.go, internal/db/repo_editor_test.go
Commit date: 2022-06-04 13:11:29 +0800
Commit message: repo_editor: check both styles of `os.PathSeparator` in all systems (#7005)


Diff: diff --git a/CHANGELOG.md b/CHANGELOG.md
index f3ecfc6c6..e09a29cce 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,10 +20,11 @@ All notable changes to Gogs are documented in this file.
 - Use [Task](https://github.com/go-task/task) as the build tool. [#6297](https://github.com/gogs/gogs/pull/6297)
 - The required Go version to compile source code changed to 1.16.
 
 ### Fixed
 
+- _Security:_ OS Command Injection in file editor. [#7000](https://github.com/gogs/gogs/issues/7000)
 - 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/db/repo_editor.go b/internal/db/repo_editor.go
index 9d4664be9..98065d439 100644
--- a/internal/db/repo_editor.go
+++ b/internal/db/repo_editor.go
@@ -465,14 +465,16 @@ type UploadRepoFileOptions struct {
 
 // isRepositoryGitPath returns true if given path is or resides inside ".git"
 // path of the repository.
 func isRepositoryGitPath(path string) bool {
 	return strings.HasSuffix(path, ".git") ||
-		strings.Contains(path, ".git"+string(os.PathSeparator)) ||
+		strings.Contains(path, ".git/") ||
+		strings.Contains(path, `.git\`) ||
 		// Windows treats ".git." the same as ".git"
 		strings.HasSuffix(path, ".git.") ||
-		strings.Contains(path, ".git."+string(os.PathSeparator))
+		strings.Contains(path, ".git./") ||
+		strings.Contains(path, `.git.\`)
 }
 
 func (repo *Repository) UploadRepoFiles(doer *User, opts UploadRepoFileOptions) error {
 	if len(opts.Files) == 0 {
 		return nil
diff --git a/internal/db/repo_editor_test.go b/internal/db/repo_editor_test.go
index ea2bf7a92..6aeed011a 100644
--- a/internal/db/repo_editor_test.go
+++ b/internal/db/repo_editor_test.go
@@ -3,37 +3,50 @@
 // license that can be found in the LICENSE file.
 
 package db
 
 import (
-	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 )
 
 func Test_isRepositoryGitPath(t *testing.T) {
 	tests := []struct {
 		path    string
 		wantVal bool
 	}{
-		{path: filepath.Join(".", ".git"), wantVal: true},
-		{path: filepath.Join(".", ".git", ""), wantVal: true},
-		{path: filepath.Join(".", ".git", "hooks", "pre-commit"), wantVal: true},
-		{path: filepath.Join(".git", "hooks"), wantVal: true},
-		{path: filepath.Join("dir", ".git"), wantVal: true},
-
-		{path: filepath.Join(".", ".git."), wantVal: true},
-		{path: filepath.Join(".", ".git.", ""), wantVal: true},
-		{path: filepath.Join(".", ".git.", "hooks", "pre-commit"), wantVal: true},
-		{path: filepath.Join(".git.", "hooks"), wantVal: true},
-		{path: filepath.Join("dir", ".git."), wantVal: true},
-
-		{path: filepath.Join(".gitignore"), wantVal: false},
-		{path: filepath.Join("dir", ".gitkeep"), wantVal: false},
+		{path: ".git", wantVal: true},
+		{path: "./.git", wantVal: true},
+		{path: ".git/hooks/pre-commit", wantVal: true},
+		{path: ".git/hooks", wantVal: true},
+		{path: "dir/.git", wantVal: true},
+
+		{path: ".gitignore", wantVal: false},
+		{path: "dir/.gitkeep", wantVal: false},
+
+		// Windows-specific
+		{path: `.git\`, wantVal: true},
+		{path: `.git\hooks\pre-commit`, wantVal: true},
+		{path: `.git\hooks`, wantVal: true},
+		{path: `dir\.git`, wantVal: true},
+
+		{path: `.\.git.`, wantVal: true},
+		{path: `.\.git.\`, wantVal: true},
+		{path: `.git.\hooks\pre-commit`, wantVal: true},
+		{path: `.git.\hooks`, wantVal: true},
+		{path: `dir\.git.`, wantVal: true},
+
+		{path: "./.git.", wantVal: true},
+		{path: "./.git./", wantVal: true},
+		{path: ".git./hooks/pre-commit", wantVal: true},
+		{path: ".git./hooks", wantVal: true},
+		{path: "dir/.git.", wantVal: true},
+
+		{path: `dir\.gitkeep`, wantVal: false},
 	}
 	for _, test := range tests {
-		t.Run("", func(t *testing.T) {
+		t.Run(test.path, func(t *testing.T) {
 			assert.Equal(t, test.wantVal, isRepositoryGitPath(test.path))
 		})
 	}
 }

Vulnerable code: 		strings.Contains(path, ".git"+string(os.PathSeparator)) ||
		strings.Contains(path, ".git."+string(os.PathSeparator))
	"path/filepath"
		{path: filepath.Join(".", ".git"), wantVal: true},
		{path: filepath.Join(".", ".git", ""), wantVal: true},
		{path: filepath.Join(".", ".git", "hooks", "pre-commit"), wantVal: true},
		{path: filepath.Join(".git", "hooks"), wantVal: true},
		{path: filepath.Join("dir", ".git"), wantVal: true},

		{path: filepath.Join(".", ".git."), wantVal: true},
		{path: filepath.Join(".", ".git.", ""), wantVal: true},
		{path: filepath.Join(".", ".git.", "hooks", "pre-commit"), wantVal: true},
		{path: filepath.Join(".git.", "hooks"), wantVal: true},
		{path: filepath.Join("dir", ".git."), wantVal: true},

		{path: filepath.Join(".gitignore"), wantVal: false},
		{path: filepath.Join("dir", ".gitkeep"), wantVal: false},
		t.Run("", func(t *testing.T) {
Fixed code: - _Security:_ OS Command Injection in file editor. [#7000](https://github.com/gogs/gogs/issues/7000)
		strings.Contains(path, ".git/") ||
		strings.Contains(path, `.git\`) ||
		strings.Contains(path, ".git./") ||
		strings.Contains(path, `.git.\`)
		{path: ".git", wantVal: true},
		{path: "./.git", wantVal: true},
		{path: ".git/hooks/pre-commit", wantVal: true},
		{path: ".git/hooks", wantVal: true},
		{path: "dir/.git", wantVal: true},

		{path: ".gitignore", wantVal: false},
		{path: "dir/.gitkeep", wantVal: false},

		// Windows-specific
		{path: `.git\`, wantVal: true},
		{path: `.git\hooks\pre-commit`, wantVal: true},
		{path: `.git\hooks`, wantVal: true},
		{path: `dir\.git`, wantVal: true},

		{path: `.\.git.`, wantVal: true},
		{path: `.\.git.\`, wantVal: true},
		{path: `.git.\hooks\pre-commit`, wantVal: true},
		{path: `.git.\hooks`, wantVal: true},
		{path: `dir\.git.`, wantVal: true},

		{path: "./.git.", wantVal: true},
		{path: "./.git./", wantVal: true},
		{path: ".git./hooks/pre-commit", wantVal: true},
		{path: ".git./hooks", wantVal: true},
		{path: "dir/.git.", wantVal: true},

		{path: `dir\.gitkeep`, wantVal: false},
		t.Run(test.path, func(t *testing.T) {