From 38aff73251cc46ced96dd608dab6190415032a82 Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Sat, 4 Jun 2022 13:11:29 +0800 Subject: [PATCH] repo_editor: check both styles of `os.PathSeparator` in all systems (#7005) --- CHANGELOG.md | 1 + internal/db/repo_editor.go | 6 +++-- internal/db/repo_editor_test.go | 45 +++++++++++++++++++++------------ 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3ecfc6c66..e09a29ccea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ 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) - 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) diff --git a/internal/db/repo_editor.go b/internal/db/repo_editor.go index 9d4664be9d..98065d4399 100644 --- a/internal/db/repo_editor.go +++ b/internal/db/repo_editor.go @@ -467,10 +467,12 @@ type UploadRepoFileOptions struct { // 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 { diff --git a/internal/db/repo_editor_test.go b/internal/db/repo_editor_test.go index ea2bf7a92d..6aeed011aa 100644 --- a/internal/db/repo_editor_test.go +++ b/internal/db/repo_editor_test.go @@ -5,7 +5,6 @@ package db import ( - "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -16,23 +15,37 @@ func Test_isRepositoryGitPath(t *testing.T) { 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)) }) }