-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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 <[email protected]> * missing change Signed-off-by: Andrew Thornton <[email protected]> * fix Test_CmdKeys Signed-off-by: Andrew Thornton <[email protected]>
- Loading branch information
Showing
5 changed files
with
200 additions
and
8 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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() | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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) | ||
} | ||
}) | ||
} | ||
} |