Remove environment variable parsing from SSH server

This removes the environment variable parsing code from the SSH server,
which never worked in the first place. Since environment variable
passing doesn't appear to be necessary for the built-in SSH server to
work properly, it's removed to reduce attack surface rather than fixing
it.

The current code processes (untrusted) input in a buggy manner and
passes it to a process invocation which doesn't actually do anything. I
don't *think* this is an exploitable vulnerability but I haven't looked
at it in detail, and it wouldn't really surprise me if it was.

Closes #1935, an alternative proposal which which partially fixes the
environment variable handling but ultimately still leaves it broken.

Signed-off-by: Hugo Landau <hlandau@devever.net>
This commit is contained in:
Hugo Landau 2018-06-17 22:46:10 +01:00
parent 85414d8b75
commit 4a6466cca5
No known key found for this signature in database
GPG Key ID: 3D30A3A9FF1360DC

View File

@ -1,4 +1,5 @@
// Copyright 2014 The Gogs Authors. All rights reserved.
// Copyright 2018 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.
@ -47,18 +48,6 @@ func handleServerConn(keyID string, chans <-chan ssh.NewChannel) {
for req := range in {
payload := cleanCommand(string(req.Payload))
switch req.Type {
case "env":
args := strings.Split(strings.Replace(payload, "\x00", "", -1), "\v")
if len(args) != 2 {
log.Warn("SSH: Invalid env arguments: '%#v'", args)
continue
}
args[0] = strings.TrimLeft(args[0], "\x04")
_, _, err := com.ExecCmdBytes("env", args[0]+"="+args[1])
if err != nil {
log.Error(3, "env: %v", err)
return
}
case "exec":
cmdName := strings.TrimLeft(payload, "'()")
log.Trace("SSH: Payload: %v", cmdName)