Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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)
Comment thread
techknowlogick marked this conversation as resolved.
}

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"`,
Comment thread
techknowlogick marked this conversation as resolved.
}, {
"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)
}
})
}
}