Skip to content

Completely quote AppPath and CustomConf paths #12955

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 29, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions integrations/cmd_keys_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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"},
}
Expand Down
6 changes: 3 additions & 3 deletions models/ssh_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions modules/repository/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
100 changes: 100 additions & 0 deletions modules/util/shellquote.go
Original file line number Diff line number Diff line change
@@ -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()
}
92 changes: 92 additions & 0 deletions modules/util/shellquote_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}