diff options
| -rw-r--r-- | integrations/cmd_keys_test.go | 4 | ||||
| -rw-r--r-- | models/ssh_key.go | 6 | ||||
| -rw-r--r-- | modules/repository/hooks.go | 6 | ||||
| -rw-r--r-- | modules/util/shellquote.go | 100 | ||||
| -rw-r--r-- | modules/util/shellquote_test.go | 92 |
5 files changed, 200 insertions, 8 deletions
diff --git a/integrations/cmd_keys_test.go b/integrations/cmd_keys_test.go index 64867d6e4c..27ae20cccd 100644 --- a/integrations/cmd_keys_test.go +++ b/integrations/cmd_keys_test.go @@ -10,11 +10,11 @@ import ( "io" "net/url" "os" - "strconv" "testing" "code.gitea.io/gitea/cmd" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "github.com/urfave/cli" ) @@ -32,7 +32,7 @@ func Test_CmdKeys(t *testing.T) { {"with_key", []string{"keys", "-e", "git", "-u", "git", "-t", "ssh-rsa", "-k", "AAAAB3NzaC1yc2EAAAADAQABAAABgQDWVj0fQ5N8wNc0LVNA41wDLYJ89ZIbejrPfg/avyj3u/ZohAKsQclxG4Ju0VirduBFF9EOiuxoiFBRr3xRpqzpsZtnMPkWVWb+akZwBFAx8p+jKdy4QXR/SZqbVobrGwip2UjSrri1CtBxpJikojRIZfCnDaMOyd9Jp6KkujvniFzUWdLmCPxUE9zhTaPu0JsEP7MW0m6yx7ZUhHyfss+NtqmFTaDO+QlMR7L2QkDliN2Jl3Xa3PhuWnKJfWhdAq1Cw4oraKUOmIgXLkuiuxVQ6mD3AiFupkmfqdHq6h+uHHmyQqv3gU+/sD8GbGAhf6ftqhTsXjnv1Aj4R8NoDf9BS6KRkzkeun5UisSzgtfQzjOMEiJtmrep2ZQrMGahrXa+q4VKr0aKJfm+KlLfwm/JztfsBcqQWNcTURiCFqz+fgZw0Ey/de0eyMzldYTdXXNRYCKjs9bvBK+6SSXRM7AhftfQ0ZuoW5+gtinPrnmoOaSCEJbAiEiTO/BzOHgowiM="}, false, - "# gitea public key\ncommand=\"" + setting.AppPath + " --config=" + strconv.Quote(strconv.Quote(setting.CustomConf))[1:len(strconv.Quote(strconv.Quote(setting.CustomConf)))-1] + " serv key-1\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDWVj0fQ5N8wNc0LVNA41wDLYJ89ZIbejrPfg/avyj3u/ZohAKsQclxG4Ju0VirduBFF9EOiuxoiFBRr3xRpqzpsZtnMPkWVWb+akZwBFAx8p+jKdy4QXR/SZqbVobrGwip2UjSrri1CtBxpJikojRIZfCnDaMOyd9Jp6KkujvniFzUWdLmCPxUE9zhTaPu0JsEP7MW0m6yx7ZUhHyfss+NtqmFTaDO+QlMR7L2QkDliN2Jl3Xa3PhuWnKJfWhdAq1Cw4oraKUOmIgXLkuiuxVQ6mD3AiFupkmfqdHq6h+uHHmyQqv3gU+/sD8GbGAhf6ftqhTsXjnv1Aj4R8NoDf9BS6KRkzkeun5UisSzgtfQzjOMEiJtmrep2ZQrMGahrXa+q4VKr0aKJfm+KlLfwm/JztfsBcqQWNcTURiCFqz+fgZw0Ey/de0eyMzldYTdXXNRYCKjs9bvBK+6SSXRM7AhftfQ0ZuoW5+gtinPrnmoOaSCEJbAiEiTO/BzOHgowiM= user2@localhost\n", + "# gitea public key\ncommand=\"" + setting.AppPath + " --config=" + util.ShellEscape(setting.CustomConf) + " serv key-1\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDWVj0fQ5N8wNc0LVNA41wDLYJ89ZIbejrPfg/avyj3u/ZohAKsQclxG4Ju0VirduBFF9EOiuxoiFBRr3xRpqzpsZtnMPkWVWb+akZwBFAx8p+jKdy4QXR/SZqbVobrGwip2UjSrri1CtBxpJikojRIZfCnDaMOyd9Jp6KkujvniFzUWdLmCPxUE9zhTaPu0JsEP7MW0m6yx7ZUhHyfss+NtqmFTaDO+QlMR7L2QkDliN2Jl3Xa3PhuWnKJfWhdAq1Cw4oraKUOmIgXLkuiuxVQ6mD3AiFupkmfqdHq6h+uHHmyQqv3gU+/sD8GbGAhf6ftqhTsXjnv1Aj4R8NoDf9BS6KRkzkeun5UisSzgtfQzjOMEiJtmrep2ZQrMGahrXa+q4VKr0aKJfm+KlLfwm/JztfsBcqQWNcTURiCFqz+fgZw0Ey/de0eyMzldYTdXXNRYCKjs9bvBK+6SSXRM7AhftfQ0ZuoW5+gtinPrnmoOaSCEJbAiEiTO/BzOHgowiM= user2@localhost\n", }, {"invalid", []string{"keys", "--not-a-flag=git"}, true, "Incorrect Usage: flag provided but not defined: -not-a-flag\n\n"}, } diff --git a/models/ssh_key.go b/models/ssh_key.go index 753ad57934..b46ff76b94 100644 --- a/models/ssh_key.go +++ b/models/ssh_key.go @@ -38,8 +38,8 @@ import ( const ( tplCommentPrefix = `# gitea public key` - tplCommand = "%s --config=%q serv key-%d" - tplPublicKey = tplCommentPrefix + "\n" + `command=%q,no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty %s` + "\n" + tplCommand = "%s --config=%s serv key-%d" + tplPublicKey = tplCommentPrefix + "\n" + `command=%s,no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty %s` + "\n" ) var sshOpLocker sync.Mutex @@ -84,7 +84,7 @@ func (key *PublicKey) OmitEmail() string { // AuthorizedString returns formatted public key string for authorized_keys file. func (key *PublicKey) AuthorizedString() string { - return fmt.Sprintf(tplPublicKey, fmt.Sprintf(tplCommand, setting.AppPath, setting.CustomConf, key.ID), key.Content) + return fmt.Sprintf(tplPublicKey, util.ShellEscape(fmt.Sprintf(tplCommand, util.ShellEscape(setting.AppPath), util.ShellEscape(setting.CustomConf), key.ID)), key.Content) } func extractTypeFromBase64Key(key string) (string, error) { 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) + } + }) + } +} |