From 4c6ac08182b5a14eaaffaafafef160bd90c4ae81 Mon Sep 17 00:00:00 2001
From: zeripath <art27@cantab.net>
Date: Tue, 29 Sep 2020 02:16:52 +0100
Subject: Completely quote AppPath and CustomConf paths (#12955)
* Completely quote AppPath and CustomConf paths
Properly handle spaces in AppPath and CustomConf within hooks and
authorized_keys. Unfortunately here we don't seem to be able to get away
with using go-shellquote as it appears that Windows doesn't play too
well with singlequote quoting - therefore we will avoid singlequote
quoting unless we absolutely cannot get away without it, e.g. \n or !.
Fix #10813
Signed-off-by: Andrew Thornton <art27@cantab.net>
* missing change
Signed-off-by: Andrew Thornton <art27@cantab.net>
* fix Test_CmdKeys
Signed-off-by: Andrew Thornton <art27@cantab.net>
---
modules/repository/hooks.go | 6 +--
modules/util/shellquote.go | 100 ++++++++++++++++++++++++++++++++++++++++
modules/util/shellquote_test.go | 92 ++++++++++++++++++++++++++++++++++++
3 files changed, 195 insertions(+), 3 deletions(-)
create mode 100644 modules/util/shellquote.go
create mode 100644 modules/util/shellquote_test.go
(limited to 'modules')
diff --git a/modules/repository/hooks.go b/modules/repository/hooks.go
index 2cefd56069..faf9c98f8a 100644
--- a/modules/repository/hooks.go
+++ b/modules/repository/hooks.go
@@ -28,9 +28,9 @@ func getHookTemplates() (hookNames, hookTpls, giteaHookTpls []string) {
fmt.Sprintf("#!/usr/bin/env %s\ndata=$(cat)\nexitcodes=\"\"\nhookname=$(basename $0)\nGIT_DIR=${GIT_DIR:-$(dirname $0)}\n\nfor hook in ${GIT_DIR}/hooks/${hookname}.d/*; do\ntest -x \"${hook}\" && test -f \"${hook}\" || continue\necho \"${data}\" | \"${hook}\"\nexitcodes=\"${exitcodes} $?\"\ndone\n\nfor i in ${exitcodes}; do\n[ ${i} -eq 0 ] || exit ${i}\ndone\n", setting.ScriptType),
}
giteaHookTpls = []string{
- fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' pre-receive\n", setting.ScriptType, setting.AppPath, setting.CustomConf),
- fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' update $1 $2 $3\n", setting.ScriptType, setting.AppPath, setting.CustomConf),
- fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' post-receive\n", setting.ScriptType, setting.AppPath, setting.CustomConf),
+ fmt.Sprintf("#!/usr/bin/env %s\n%s hook --config=%s pre-receive\n", setting.ScriptType, util.ShellEscape(setting.AppPath), util.ShellEscape(setting.CustomConf)),
+ fmt.Sprintf("#!/usr/bin/env %s\n%s hook --config=%s update $1 $2 $3\n", setting.ScriptType, util.ShellEscape(setting.AppPath), util.ShellEscape(setting.CustomConf)),
+ fmt.Sprintf("#!/usr/bin/env %s\n%s hook --config=%s post-receive\n", setting.ScriptType, util.ShellEscape(setting.AppPath), util.ShellEscape(setting.CustomConf)),
}
return
}
diff --git a/modules/util/shellquote.go b/modules/util/shellquote.go
new file mode 100644
index 0000000000..bde24a1517
--- /dev/null
+++ b/modules/util/shellquote.go
@@ -0,0 +1,100 @@
+// Copyright 2020 The Gitea 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 util
+
+import "strings"
+
+// Bash has the definition of a metacharacter:
+// * A character that, when unquoted, separates words.
+// A metacharacter is one of: " \t\n|&;()<>"
+//
+// The following characters also have addition special meaning when unescaped:
+// * ‘${[*?!"'`\’
+//
+// Double Quotes preserve the literal value of all characters with then quotes
+// excepting: ‘$’, ‘`’, ‘\’, and, when history expansion is enabled, ‘!’.
+// The backslash retains its special meaning only when followed by one of the
+// following characters: ‘$’, ‘`’, ‘"’, ‘\’, or newline.
+// Backslashes preceding characters without a special meaning are left
+// unmodified. A double quote may be quoted within double quotes by preceding
+// it with a backslash. If enabled, history expansion will be performed unless
+// an ‘!’ appearing in double quotes is escaped using a backslash. The
+// backslash preceding the ‘!’ is not removed.
+//
+// -> This means that `!\n` cannot be safely expressed in `"`.
+//
+// Looking at the man page for Dash and ash the situation is similar.
+//
+// Now zsh requires that ‘}’, and ‘]’ are also enclosed in doublequotes or escaped
+//
+// Single quotes escape everything except a ‘'’
+//
+// There's one other gotcha - ‘~’ at the start of a string needs to be expanded
+// because people always expect that - of course if there is a special character before '/'
+// this is not going to work
+
+const (
+ tildePrefix = '~'
+ needsEscape = " \t\n|&;()<>${}[]*?!\"'`\\"
+ needsSingleQuote = "!\n"
+)
+
+var doubleQuoteEscaper = strings.NewReplacer(`$`, `\$`, "`", "\\`", `"`, `\"`, `\`, `\\`)
+var singleQuoteEscaper = strings.NewReplacer(`'`, `'\''`)
+var singleQuoteCoalescer = strings.NewReplacer(`''\'`, `\'`, `\'''`, `\'`)
+
+// ShellEscape will escape the provided string.
+// We can't just use go-shellquote here because our preferences for escaping differ from those in that we want:
+//
+// * If the string doesn't require any escaping just leave it as it is.
+// * If the string requires any escaping prefer double quote escaping
+// * If we have ! or newlines then we need to use single quote escaping
+func ShellEscape(toEscape string) string {
+ if len(toEscape) == 0 {
+ return toEscape
+ }
+
+ start := 0
+
+ if toEscape[0] == tildePrefix {
+ // We're in the forcibly non-escaped section...
+ idx := strings.IndexRune(toEscape, '/')
+ if idx < 0 {
+ idx = len(toEscape)
+ } else {
+ idx++
+ }
+ if !strings.ContainsAny(toEscape[:idx], needsEscape) {
+ // We'll assume that they intend ~ expansion to occur
+ start = idx
+ }
+ }
+
+ // Now for simplicity we'll look at the rest of the string
+ if !strings.ContainsAny(toEscape[start:], needsEscape) {
+ return toEscape
+ }
+
+ // OK we have to do some escaping
+ sb := &strings.Builder{}
+ _, _ = sb.WriteString(toEscape[:start])
+
+ // Do we have any characters which absolutely need to be within single quotes - that is simply ! or \n?
+ if strings.ContainsAny(toEscape[start:], needsSingleQuote) {
+ // We need to single quote escape.
+ sb2 := &strings.Builder{}
+ _, _ = sb2.WriteRune('\'')
+ _, _ = singleQuoteEscaper.WriteString(sb2, toEscape[start:])
+ _, _ = sb2.WriteRune('\'')
+ _, _ = singleQuoteCoalescer.WriteString(sb, sb2.String())
+ return sb.String()
+ }
+
+ // OK we can just use " just escape the things that need escaping
+ _, _ = sb.WriteRune('"')
+ _, _ = doubleQuoteEscaper.WriteString(sb, toEscape[start:])
+ _, _ = sb.WriteRune('"')
+ return sb.String()
+}
diff --git a/modules/util/shellquote_test.go b/modules/util/shellquote_test.go
new file mode 100644
index 0000000000..2ddc6d763d
--- /dev/null
+++ b/modules/util/shellquote_test.go
@@ -0,0 +1,92 @@
+// Copyright 2020 The Gitea 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 util
+
+import "testing"
+
+func TestShellEscape(t *testing.T) {
+ tests := []struct {
+ name string
+ toEscape string
+ want string
+ }{
+ {
+ "Simplest case - nothing to escape",
+ "a/b/c/d",
+ "a/b/c/d",
+ }, {
+ "Prefixed tilde - with normal stuff - should not escape",
+ "~/src/go/gitea/gitea",
+ "~/src/go/gitea/gitea",
+ }, {
+ "Typical windows path with spaces - should get doublequote escaped",
+ `C:\Program Files\Gitea v1.13 - I like lots of spaces\gitea`,
+ `"C:\\Program Files\\Gitea v1.13 - I like lots of spaces\\gitea"`,
+ }, {
+ "Forward-slashed windows path with spaces - should get doublequote escaped",
+ "C:/Program Files/Gitea v1.13 - I like lots of spaces/gitea",
+ `"C:/Program Files/Gitea v1.13 - I like lots of spaces/gitea"`,
+ }, {
+ "Prefixed tilde - but then a space filled path",
+ "~git/Gitea v1.13/gitea",
+ `~git/"Gitea v1.13/gitea"`,
+ }, {
+ "Bangs are unforutunately not predictable so need to be singlequoted",
+ "C:/Program Files/Gitea!/gitea",
+ `'C:/Program Files/Gitea!/gitea'`,
+ }, {
+ "Newlines are just irritating",
+ "/home/git/Gitea\n\nWHY-WOULD-YOU-DO-THIS\n\nGitea/gitea",
+ "'/home/git/Gitea\n\nWHY-WOULD-YOU-DO-THIS\n\nGitea/gitea'",
+ }, {
+ "Similarly we should nicely handle mutiple single quotes if we have to single-quote",
+ "'!''!'''!''!'!'",
+ `\''!'\'\''!'\'\'\''!'\'\''!'\''!'\'`,
+ }, {
+ "Double quote < ...",
+ "~/<gitea",
+ "~/\"<gitea\"",
+ }, {
+ "Double quote > ...",
+ "~/gitea>",
+ "~/\"gitea>\"",
+ }, {
+ "Double quote and escape $ ...",
+ "~/$gitea",
+ "~/\"\\$gitea\"",
+ }, {
+ "Double quote {...",
+ "~/{gitea",
+ "~/\"{gitea\"",
+ }, {
+ "Double quote }...",
+ "~/gitea}",
+ "~/\"gitea}\"",
+ }, {
+ "Double quote ()...",
+ "~/(gitea)",
+ "~/\"(gitea)\"",
+ }, {
+ "Double quote and escape `...",
+ "~/gitea`",
+ "~/\"gitea\\`\"",
+ }, {
+ "Double quotes can handle a number of things without having to escape them but not everything ...",
+ "~/<gitea> ${gitea} `gitea` [gitea] (gitea) \"gitea\" \\gitea\\ 'gitea'",
+ "~/\"<gitea> \\${gitea} \\`gitea\\` [gitea] (gitea) \\\"gitea\\\" \\\\gitea\\\\ 'gitea'\"",
+ }, {
+ "Single quotes don't need to escape except for '...",
+ "~/<gitea> ${gitea} `gitea` (gitea) !gitea! \"gitea\" \\gitea\\ 'gitea'",
+ "~/'<gitea> ${gitea} `gitea` (gitea) !gitea! \"gitea\" \\gitea\\ '\\''gitea'\\'",
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ if got := ShellEscape(tt.toEscape); got != tt.want {
+ t.Errorf("ShellEscape(%q):\nGot: %s\nWanted: %s", tt.toEscape, got, tt.want)
+ }
+ })
+ }
+}
--
cgit v1.2.3