diff options
author | Anderson Toshiyuki Sasaki <ansasaki@redhat.com> | 2019-10-31 18:10:27 +0100 |
---|---|---|
committer | Andreas Schneider <asn@cryptomilk.org> | 2019-12-09 17:34:30 +0100 |
commit | b0edec4e8d01ad73b0d26ad4070d7e1a1e86dfc8 (patch) | |
tree | 44224babda47c8f87080077be51cf37d49e71d0b | |
parent | 391c78de9d0f7baec3a44d86a76f4e1324eb9529 (diff) | |
download | libssh-b0edec4e8d01ad73b0d26ad4070d7e1a1e86dfc8.tar.gz libssh-b0edec4e8d01ad73b0d26ad4070d7e1a1e86dfc8.tar.xz libssh-b0edec4e8d01ad73b0d26ad4070d7e1a1e86dfc8.zip |
CVE-2019-14889: scp: Quote location to be used on shell
Single quote file paths to be used on commands to be executed on remote
shell.
Fixes T181
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
(cherry picked from commit 3830c7ae6eec751b7618d3fc159cb5bb3c8806a6)
-rw-r--r-- | src/scp.c | 62 |
1 files changed, 56 insertions, 6 deletions
@@ -29,6 +29,7 @@ #include "libssh/priv.h" #include "libssh/scp.h" +#include "libssh/misc.h" /** * @defgroup libssh_scp The SSH scp functions @@ -119,6 +120,9 @@ int ssh_scp_init(ssh_scp scp) { int rc; char execbuffer[1024] = {0}; + char *quoted_location = NULL; + size_t quoted_location_len = 0; + size_t scp_location_len; if (scp == NULL) { return SSH_ERROR; @@ -130,33 +134,79 @@ int ssh_scp_init(ssh_scp scp) return SSH_ERROR; } - SSH_LOG(SSH_LOG_PROTOCOL, - "Initializing scp session %s %son location '%s'", + if (scp->location == NULL) { + ssh_set_error(scp->session, SSH_FATAL, + "Invalid scp context: location is NULL"); + return SSH_ERROR; + } + + SSH_LOG(SSH_LOG_PROTOCOL, "Initializing scp session %s %son location '%s'", scp->mode == SSH_SCP_WRITE?"write":"read", - scp->recursive?"recursive ":"", + scp->recursive ? "recursive " : "", scp->location); scp->channel = ssh_channel_new(scp->session); if (scp->channel == NULL) { + ssh_set_error(scp->session, SSH_FATAL, + "Channel creation failed for scp"); scp->state = SSH_SCP_ERROR; return SSH_ERROR; } rc = ssh_channel_open_session(scp->channel); if (rc == SSH_ERROR) { + ssh_set_error(scp->session, SSH_FATAL, + "Failed to open channel for scp"); + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + /* In the worst case, each character would be replaced by 3 plus the string + * terminator '\0' */ + scp_location_len = strlen(scp->location); + quoted_location_len = ((size_t)3 * scp_location_len) + 1; + /* Paranoia check */ + if (quoted_location_len < scp_location_len) { + ssh_set_error(scp->session, SSH_FATAL, + "Buffer overflow detected"); + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + quoted_location = (char *)calloc(1, quoted_location_len); + if (quoted_location == NULL) { + ssh_set_error(scp->session, SSH_FATAL, + "Failed to allocate memory for quoted location"); + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + rc = ssh_quote_file_name(scp->location, quoted_location, + quoted_location_len); + if (rc <= 0) { + ssh_set_error(scp->session, SSH_FATAL, + "Failed to single quote command location"); + SAFE_FREE(quoted_location); scp->state = SSH_SCP_ERROR; return SSH_ERROR; } if (scp->mode == SSH_SCP_WRITE) { snprintf(execbuffer, sizeof(execbuffer), "scp -t %s %s", - scp->recursive ? "-r":"", scp->location); + scp->recursive ? "-r" : "", quoted_location); } else { snprintf(execbuffer, sizeof(execbuffer), "scp -f %s %s", - scp->recursive ? "-r":"", scp->location); + scp->recursive ? "-r" : "", quoted_location); } - if (ssh_channel_request_exec(scp->channel, execbuffer) == SSH_ERROR) { + SAFE_FREE(quoted_location); + + SSH_LOG(SSH_LOG_DEBUG, "Executing command: %s", execbuffer); + + rc = ssh_channel_request_exec(scp->channel, execbuffer); + if (rc == SSH_ERROR){ + ssh_set_error(scp->session, SSH_FATAL, + "Failed executing command: %s", execbuffer); scp->state = SSH_SCP_ERROR; return SSH_ERROR; } |