Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-15-SP3:Update
libssh.13500
CVE-2019-14889.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File CVE-2019-14889.patch of Package libssh.13500
From b441faa7e66a33b00090e345119fbbbb09592c16 Mon Sep 17 00:00:00 2001 From: Anderson Toshiyuki Sasaki <ansasaki@redhat.com> Date: Thu, 31 Oct 2019 17:56:34 +0100 Subject: [PATCH 1/4] CVE-2019-14889: scp: Reformat scp.c Fixes T181 Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org> (cherry picked from commit 8cb53e5c7a3ed0738ca38746cd282296ac4c882e) (cherry picked from commit 3cecb754c3a577c2b77ecf776921e350c6a221f3) --- src/scp.c | 1200 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 698 insertions(+), 502 deletions(-) diff --git a/src/scp.c b/src/scp.c index fd9aaaaa..5de0e6ff 100644 --- a/src/scp.c +++ b/src/scp.c @@ -57,30 +57,47 @@ * * @returns A ssh_scp handle, NULL if the creation was impossible. */ -ssh_scp ssh_scp_new(ssh_session session, int mode, const char *location){ - ssh_scp scp=malloc(sizeof(struct ssh_scp_struct)); - if(scp == NULL){ - ssh_set_error(session,SSH_FATAL,"Error allocating memory for ssh_scp"); - return NULL; - } - ZERO_STRUCTP(scp); - if((mode&~SSH_SCP_RECURSIVE) != SSH_SCP_WRITE && (mode &~SSH_SCP_RECURSIVE) != SSH_SCP_READ){ - ssh_set_error(session,SSH_FATAL,"Invalid mode %d for ssh_scp_new()",mode); - ssh_scp_free(scp); - return NULL; - } - scp->location=strdup(location); - if (scp->location == NULL) { - ssh_set_error(session,SSH_FATAL,"Error allocating memory for ssh_scp"); +ssh_scp ssh_scp_new(ssh_session session, int mode, const char *location) +{ + ssh_scp scp = NULL; + + if (session == NULL) { + goto error; + } + + scp = (ssh_scp)calloc(1, sizeof(struct ssh_scp_struct)); + if (scp == NULL) { + ssh_set_error(session, SSH_FATAL, + "Error allocating memory for ssh_scp"); + goto error; + } + + if ((mode & ~SSH_SCP_RECURSIVE) != SSH_SCP_WRITE && + (mode & ~SSH_SCP_RECURSIVE) != SSH_SCP_READ) + { + ssh_set_error(session, SSH_FATAL, + "Invalid mode %d for ssh_scp_new()", mode); + goto error; + } + + scp->location = strdup(location); + if (scp->location == NULL) { + ssh_set_error(session, SSH_FATAL, + "Error allocating memory for ssh_scp"); + goto error; + } + + scp->session = session; + scp->mode = mode & ~SSH_SCP_RECURSIVE; + scp->recursive = (mode & SSH_SCP_RECURSIVE) != 0; + scp->channel = NULL; + scp->state = SSH_SCP_NEW; + + return scp; + +error: ssh_scp_free(scp); return NULL; - } - scp->session=session; - scp->mode=mode & ~SSH_SCP_RECURSIVE; - scp->recursive = (mode & SSH_SCP_RECURSIVE) != 0; - scp->channel=NULL; - scp->state=SSH_SCP_NEW; - return scp; } /** @@ -94,59 +111,78 @@ ssh_scp ssh_scp_new(ssh_session session, int mode, const char *location){ */ int ssh_scp_init(ssh_scp scp) { - int r; - char execbuffer[1024]; - uint8_t code; - if(scp==NULL) - return SSH_ERROR; - if(scp->state != SSH_SCP_NEW){ - ssh_set_error(scp->session,SSH_FATAL,"ssh_scp_init called under invalid state"); - 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->location); - scp->channel=ssh_channel_new(scp->session); - if(scp->channel == NULL){ - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; - } - r= ssh_channel_open_session(scp->channel); - if(r==SSH_ERROR){ - 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); - else - snprintf(execbuffer,sizeof(execbuffer),"scp -f %s %s", - scp->recursive ? "-r":"", scp->location); - if(ssh_channel_request_exec(scp->channel,execbuffer) == SSH_ERROR){ - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; - } - if(scp->mode == SSH_SCP_WRITE){ - r=ssh_channel_read(scp->channel,&code,1,0); - if(r<=0){ - ssh_set_error(scp->session,SSH_FATAL, "Error reading status code: %s",ssh_get_error(scp->session)); - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; - } - if(code != 0){ - ssh_set_error(scp->session,SSH_FATAL, "scp status code %ud not valid", code); - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; - } - } else { - ssh_channel_write(scp->channel,"",1); - } - if(scp->mode == SSH_SCP_WRITE) - scp->state=SSH_SCP_WRITE_INITED; - else - scp->state=SSH_SCP_READ_INITED; - return SSH_OK; + int rc; + char execbuffer[1024] = {0}; + uint8_t code; + + if (scp == NULL) { + return SSH_ERROR; + } + + if (scp->state != SSH_SCP_NEW) { + ssh_set_error(scp->session, SSH_FATAL, + "ssh_scp_init called under invalid state"); + 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->location); + + scp->channel = ssh_channel_new(scp->session); + if (scp->channel == NULL) { + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + rc = ssh_channel_open_session(scp->channel); + if (rc == SSH_ERROR) { + 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); + } else { + snprintf(execbuffer, sizeof(execbuffer), "scp -f %s %s", + scp->recursive ? "-r":"", scp->location); + } + + if (ssh_channel_request_exec(scp->channel, execbuffer) == SSH_ERROR) { + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + if (scp->mode == SSH_SCP_WRITE) { + rc = ssh_channel_read(scp->channel, &code, 1, 0); + if (rc <= 0) { + ssh_set_error(scp->session, SSH_FATAL, + "Error reading status code: %s", + ssh_get_error(scp->session)); + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + if (code != 0) { + ssh_set_error(scp->session, SSH_FATAL, + "scp status code %ud not valid", code); + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + } else { + ssh_channel_write(scp->channel, "", 1); + } + + if (scp->mode == SSH_SCP_WRITE) { + scp->state = SSH_SCP_WRITE_INITED; + } else { + scp->state = SSH_SCP_READ_INITED; + } + + return SSH_OK; } /** @@ -160,33 +196,40 @@ int ssh_scp_init(ssh_scp scp) */ int ssh_scp_close(ssh_scp scp) { - char buffer[128]; - int err; - if(scp==NULL) - return SSH_ERROR; - if(scp->channel != NULL){ - if(ssh_channel_send_eof(scp->channel) == SSH_ERROR){ - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; - } - /* avoid situations where data are buffered and - * not yet stored on disk. This can happen if the close is sent - * before we got the EOF back - */ - while(!ssh_channel_is_eof(scp->channel)){ - err=ssh_channel_read(scp->channel,buffer,sizeof(buffer),0); - if(err==SSH_ERROR || err==0) - break; + char buffer[128] = {0}; + int rc; + + if (scp == NULL) { + return SSH_ERROR; } - if(ssh_channel_close(scp->channel) == SSH_ERROR){ - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; + + if (scp->channel != NULL) { + if (ssh_channel_send_eof(scp->channel) == SSH_ERROR) { + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + /* avoid situations where data are buffered and + * not yet stored on disk. This can happen if the close is sent + * before we got the EOF back + */ + while (!ssh_channel_is_eof(scp->channel)) { + rc = ssh_channel_read(scp->channel, buffer, sizeof(buffer), 0); + if (rc == SSH_ERROR || rc == 0) { + break; + } + } + + if (ssh_channel_close(scp->channel) == SSH_ERROR) { + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + ssh_channel_free(scp->channel); + scp->channel = NULL; } - ssh_channel_free(scp->channel); - scp->channel=NULL; - } - scp->state=SSH_SCP_NEW; - return SSH_OK; + + scp->state = SSH_SCP_NEW; + return SSH_OK; } /** @@ -198,16 +241,22 @@ int ssh_scp_close(ssh_scp scp) */ void ssh_scp_free(ssh_scp scp) { - if(scp==NULL) - return; - if(scp->state != SSH_SCP_NEW) - ssh_scp_close(scp); - if(scp->channel) - ssh_channel_free(scp->channel); - SAFE_FREE(scp->location); - SAFE_FREE(scp->request_name); - SAFE_FREE(scp->warning); - SAFE_FREE(scp); + if (scp == NULL) { + return; + } + + if (scp->state != SSH_SCP_NEW) { + ssh_scp_close(scp); + } + + if (scp->channel) { + ssh_channel_free(scp->channel); + } + + SAFE_FREE(scp->location); + SAFE_FREE(scp->request_name); + SAFE_FREE(scp->warning); + SAFE_FREE(scp); } /** @@ -224,81 +273,106 @@ void ssh_scp_free(ssh_scp scp) * * @see ssh_scp_leave_directory() */ -int ssh_scp_push_directory(ssh_scp scp, const char *dirname, int mode){ - char buffer[1024]; - int r; - uint8_t code; - char *dir; - char *perms; - if(scp==NULL) - return SSH_ERROR; - if(scp->state != SSH_SCP_WRITE_INITED){ - ssh_set_error(scp->session,SSH_FATAL,"ssh_scp_push_directory called under invalid state"); - return SSH_ERROR; - } - dir=ssh_basename(dirname); - perms=ssh_scp_string_mode(mode); - snprintf(buffer, sizeof(buffer), "D%s 0 %s\n", perms, dir); - SAFE_FREE(dir); - SAFE_FREE(perms); - r=ssh_channel_write(scp->channel,buffer,strlen(buffer)); - if(r==SSH_ERROR){ - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; - } - r=ssh_channel_read(scp->channel,&code,1,0); - if(r<=0){ - ssh_set_error(scp->session,SSH_FATAL, "Error reading status code: %s",ssh_get_error(scp->session)); - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; - } - if(code != 0){ - ssh_set_error(scp->session,SSH_FATAL, "scp status code %ud not valid", code); - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; - } - return SSH_OK; +int ssh_scp_push_directory(ssh_scp scp, const char *dirname, int mode) +{ + char buffer[1024] = {0}; + int rc; + uint8_t code; + char *dir = NULL; + char *perms = NULL; + + if (scp == NULL) { + return SSH_ERROR; + } + + if (scp->state != SSH_SCP_WRITE_INITED) { + ssh_set_error(scp->session, SSH_FATAL, + "ssh_scp_push_directory called under invalid state"); + return SSH_ERROR; + } + + dir = ssh_basename(dirname); + perms = ssh_scp_string_mode(mode); + snprintf(buffer, sizeof(buffer), "D%s 0 %s\n", perms, dir); + SAFE_FREE(dir); + SAFE_FREE(perms); + + rc = ssh_channel_write(scp->channel, buffer, strlen(buffer)); + if (rc == SSH_ERROR) { + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + rc = ssh_channel_read(scp->channel, &code, 1, 0); + if (rc <= 0) { + ssh_set_error(scp->session, SSH_FATAL, + "Error reading status code: %s", + ssh_get_error(scp->session)); + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + if (code != 0) { + ssh_set_error(scp->session, SSH_FATAL, "scp status code %ud not valid", + code); + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + return SSH_OK; } /** * @brief Leave a directory. * - * @returns SSH_OK if the directory has been left,SSH_ERROR if an + * @returns SSH_OK if the directory has been left, SSH_ERROR if an * error occured. * * @see ssh_scp_push_directory() */ - int ssh_scp_leave_directory(ssh_scp scp){ - char buffer[]="E\n"; - int r; - uint8_t code; - if(scp==NULL) - return SSH_ERROR; - if(scp->state != SSH_SCP_WRITE_INITED){ - ssh_set_error(scp->session,SSH_FATAL,"ssh_scp_leave_directory called under invalid state"); - return SSH_ERROR; - } - r=ssh_channel_write(scp->channel,buffer,strlen(buffer)); - if(r==SSH_ERROR){ - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; - } - r=ssh_channel_read(scp->channel,&code,1,0); - if(r<=0){ - ssh_set_error(scp->session,SSH_FATAL, "Error reading status code: %s",ssh_get_error(scp->session)); - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; - } - if(code != 0){ - ssh_set_error(scp->session,SSH_FATAL, "scp status code %ud not valid", code); - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; - } - return SSH_OK; +int ssh_scp_leave_directory(ssh_scp scp) +{ + char buffer[] = "E\n"; + int rc; + uint8_t code; + + if (scp == NULL) { + return SSH_ERROR; + } + + if (scp->state != SSH_SCP_WRITE_INITED) { + ssh_set_error(scp->session, SSH_FATAL, + "ssh_scp_leave_directory called under invalid state"); + return SSH_ERROR; + } + + rc = ssh_channel_write(scp->channel, buffer, strlen(buffer)); + if (rc == SSH_ERROR) { + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + rc = ssh_channel_read(scp->channel, &code, 1, 0); + if (rc <= 0) { + ssh_set_error(scp->session, SSH_FATAL, "Error reading status code: %s", + ssh_get_error(scp->session)); + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + if (code != 0) { + ssh_set_error(scp->session, SSH_FATAL, "scp status code %ud not valid", + code); + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + return SSH_OK; } /** - * @brief Initialize the sending of a file to a scp in sink mode, using a 64-bit size. + * @brief Initialize the sending of a file to a scp in sink mode, using a 64-bit + * size. * * @param[in] scp The scp handle. * @@ -314,44 +388,61 @@ int ssh_scp_push_directory(ssh_scp scp, const char *dirname, int mode){ * * @see ssh_scp_push_file() */ -int ssh_scp_push_file64(ssh_scp scp, const char *filename, uint64_t size, int mode){ - char buffer[1024]; - int r; - uint8_t code; - char *file; - char *perms; - if(scp==NULL) - return SSH_ERROR; - if(scp->state != SSH_SCP_WRITE_INITED){ - ssh_set_error(scp->session,SSH_FATAL,"ssh_scp_push_file called under invalid state"); - return SSH_ERROR; - } - file=ssh_basename(filename); - perms=ssh_scp_string_mode(mode); - SSH_LOG(SSH_LOG_PROTOCOL,"SCP pushing file %s, size %" PRIu64 " with permissions '%s'",file,size,perms); - snprintf(buffer, sizeof(buffer), "C%s %" PRIu64 " %s\n", perms, size, file); - SAFE_FREE(file); - SAFE_FREE(perms); - r=ssh_channel_write(scp->channel,buffer,strlen(buffer)); - if(r==SSH_ERROR){ - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; - } - r=ssh_channel_read(scp->channel,&code,1,0); - if(r<=0){ - ssh_set_error(scp->session,SSH_FATAL, "Error reading status code: %s",ssh_get_error(scp->session)); - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; - } - if(code != 0){ - ssh_set_error(scp->session,SSH_FATAL, "scp status code %ud not valid", code); - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; - } - scp->filelen = size; - scp->processed = 0; - scp->state=SSH_SCP_WRITE_WRITING; - return SSH_OK; +int ssh_scp_push_file64(ssh_scp scp, const char *filename, uint64_t size, + int mode) +{ + char buffer[1024] = {0}; + int rc; + char *file = NULL; + char *perms = NULL; + uint8_t code; + + if (scp == NULL) { + return SSH_ERROR; + } + + if (scp->state != SSH_SCP_WRITE_INITED) { + ssh_set_error(scp->session, SSH_FATAL, + "ssh_scp_push_file called under invalid state"); + return SSH_ERROR; + } + + file = ssh_basename(filename); + perms = ssh_scp_string_mode(mode); + SSH_LOG(SSH_LOG_PROTOCOL, + "SCP pushing file %s, size %" PRIu64 " with permissions '%s'", + file, size, perms); + snprintf(buffer, sizeof(buffer), "C%s %" PRIu64 " %s\n", perms, size, file); + SAFE_FREE(file); + SAFE_FREE(perms); + + rc = ssh_channel_write(scp->channel, buffer, strlen(buffer)); + if (rc == SSH_ERROR) { + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + rc = ssh_channel_read(scp->channel, &code, 1, 0); + if (rc <= 0) { + ssh_set_error(scp->session, SSH_FATAL, + "Error reading status code: %s", + ssh_get_error(scp->session)); + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + if (code != 0) { + ssh_set_error(scp->session, SSH_FATAL, + "scp status code %ud not valid", code); + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + scp->filelen = size; + scp->processed = 0; + scp->state = SSH_SCP_WRITE_WRITING; + + return SSH_OK; } /** @@ -369,8 +460,9 @@ int ssh_scp_push_file64(ssh_scp scp, const char *filename, uint64_t size, int mo * @returns SSH_OK if the file is ready to be sent, SSH_ERROR if an * error occured. */ -int ssh_scp_push_file(ssh_scp scp, const char *filename, size_t size, int mode){ - return ssh_scp_push_file64(scp, filename, (uint64_t) size, mode); +int ssh_scp_push_file(ssh_scp scp, const char *filename, size_t size, int mode) +{ + return ssh_scp_push_file64(scp, filename, (uint64_t) size, mode); } /** @@ -385,41 +477,60 @@ int ssh_scp_push_file(ssh_scp scp, const char *filename, size_t size, int mode){ * * @returns The return code, SSH_ERROR a error occured. */ -int ssh_scp_response(ssh_scp scp, char **response){ - unsigned char code; - int r; - char msg[128]; - if(scp==NULL) - return SSH_ERROR; - r=ssh_channel_read(scp->channel,&code,1,0); - if(r == SSH_ERROR) - return SSH_ERROR; - if(code == 0) - return 0; - if(code > 2){ - ssh_set_error(scp->session,SSH_FATAL, "SCP: invalid status code %ud received", code); - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; - } - r=ssh_scp_read_string(scp,msg,sizeof(msg)); - if(r==SSH_ERROR) - return r; - /* Warning */ - if(code == 1){ - ssh_set_error(scp->session,SSH_REQUEST_DENIED, "SCP: Warning: status code 1 received: %s", msg); - SSH_LOG(SSH_LOG_RARE,"SCP: Warning: status code 1 received: %s", msg); - if(response) - *response=strdup(msg); - return 1; - } - if(code == 2){ - ssh_set_error(scp->session,SSH_FATAL, "SCP: Error: status code 2 received: %s", msg); - if(response) - *response=strdup(msg); - return 2; - } - /* Not reached */ - return SSH_ERROR; +int ssh_scp_response(ssh_scp scp, char **response) +{ + unsigned char code; + int rc; + char msg[128] = {0}; + + if (scp == NULL) { + return SSH_ERROR; + } + + rc = ssh_channel_read(scp->channel, &code, 1, 0); + if (rc == SSH_ERROR) { + return SSH_ERROR; + } + + if (code == 0) { + return 0; + } + + if (code > 2) { + ssh_set_error(scp->session, SSH_FATAL, + "SCP: invalid status code %ud received", code); + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + rc = ssh_scp_read_string(scp, msg, sizeof(msg)); + if (rc == SSH_ERROR) { + return rc; + } + + /* Warning */ + if (code == 1) { + ssh_set_error(scp->session, SSH_REQUEST_DENIED, + "SCP: Warning: status code 1 received: %s", msg); + SSH_LOG(SSH_LOG_RARE, + "SCP: Warning: status code 1 received: %s", msg); + if (response) { + *response = strdup(msg); + } + return 1; + } + + if (code == 2) { + ssh_set_error(scp->session, SSH_FATAL, + "SCP: Error: status code 2 received: %s", msg); + if (response) { + *response = strdup(msg); + } + return 2; + } + + /* Not reached */ + return SSH_ERROR; } /** @@ -434,57 +545,72 @@ int ssh_scp_response(ssh_scp scp, char **response){ * @returns SSH_OK if the write was successful, SSH_ERROR an error * occured while writing. */ -int ssh_scp_write(ssh_scp scp, const void *buffer, size_t len){ - int w; - int r; - uint8_t code; - if(scp==NULL) - return SSH_ERROR; - if(scp->state != SSH_SCP_WRITE_WRITING){ - ssh_set_error(scp->session,SSH_FATAL,"ssh_scp_write called under invalid state"); - return SSH_ERROR; - } - if(scp->processed + len > scp->filelen) - len = (size_t) (scp->filelen - scp->processed); - /* hack to avoid waiting for window change */ - r = ssh_channel_poll(scp->channel, 0); - if (r == SSH_ERROR) { - scp->state = SSH_SCP_ERROR; - return SSH_ERROR; - } - w=ssh_channel_write(scp->channel,buffer,len); - if(w != SSH_ERROR) - scp->processed += w; - else { - scp->state=SSH_SCP_ERROR; - //return=channel_get_exit_status(scp->channel); - return SSH_ERROR; - } - /* Far end sometimes send a status message, which we need to read - * and handle */ - r = ssh_channel_poll(scp->channel,0); - if(r > 0){ - r = ssh_channel_read(scp->channel, &code, 1, 0); - if(r == SSH_ERROR){ - return SSH_ERROR; - } - if(code == 1 || code == 2){ - ssh_set_error(scp->session,SSH_REQUEST_DENIED, "SCP: Error: status code %i received", code); - return SSH_ERROR; - } - } - /* Check if we arrived at end of file */ - if(scp->processed == scp->filelen) { - code = 0; - w = ssh_channel_write(scp->channel, &code, 1); - if(w == SSH_ERROR){ - scp->state = SSH_SCP_ERROR; - return SSH_ERROR; - } - scp->processed=scp->filelen=0; - scp->state=SSH_SCP_WRITE_INITED; - } - return SSH_OK; +int ssh_scp_write(ssh_scp scp, const void *buffer, size_t len) +{ + int w; + int rc; + uint8_t code; + + if (scp == NULL) { + return SSH_ERROR; + } + + if (scp->state != SSH_SCP_WRITE_WRITING) { + ssh_set_error(scp->session, SSH_FATAL, + "ssh_scp_write called under invalid state"); + return SSH_ERROR; + } + + if (scp->processed + len > scp->filelen) { + len = (size_t) (scp->filelen - scp->processed); + } + + /* hack to avoid waiting for window change */ + rc = ssh_channel_poll(scp->channel, 0); + if (rc == SSH_ERROR) { + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + w = ssh_channel_write(scp->channel, buffer, len); + if (w != SSH_ERROR) { + scp->processed += w; + } else { + scp->state = SSH_SCP_ERROR; + //return = channel_get_exit_status(scp->channel); + return SSH_ERROR; + } + + /* Far end sometimes send a status message, which we need to read + * and handle */ + rc = ssh_channel_poll(scp->channel, 0); + if (rc > 0) { + rc = ssh_channel_read(scp->channel, &code, 1, 0); + if (rc == SSH_ERROR) { + return SSH_ERROR; + } + + if (code == 1 || code == 2) { + ssh_set_error(scp->session, SSH_REQUEST_DENIED, + "SCP: Error: status code %i received", code); + return SSH_ERROR; + } + } + + /* Check if we arrived at end of file */ + if (scp->processed == scp->filelen) { + code = 0; + w = ssh_channel_write(scp->channel, &code, 1); + if (w == SSH_ERROR) { + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + scp->processed = scp->filelen = 0; + scp->state = SSH_SCP_WRITE_INITED; + } + + return SSH_OK; } /** @@ -501,27 +627,36 @@ int ssh_scp_write(ssh_scp scp, const void *buffer, size_t len){ * @returns SSH_OK if the string was read, SSH_ERROR if an error * occured while reading. */ -int ssh_scp_read_string(ssh_scp scp, char *buffer, size_t len){ - size_t r=0; - int err=SSH_OK; - if(scp==NULL) - return SSH_ERROR; - while(r<len-1){ - err=ssh_channel_read(scp->channel,&buffer[r],1,0); - if(err==SSH_ERROR){ - break; - } - if(err==0){ - ssh_set_error(scp->session,SSH_FATAL,"End of file while reading string"); - err=SSH_ERROR; - break; - } - r++; - if(buffer[r-1] == '\n') - break; - } - buffer[r]=0; - return err; +int ssh_scp_read_string(ssh_scp scp, char *buffer, size_t len) +{ + size_t read = 0; + int err = SSH_OK; + + if (scp == NULL) { + return SSH_ERROR; + } + + while (read < len - 1) { + err = ssh_channel_read(scp->channel, &buffer[read], 1, 0); + if (err == SSH_ERROR) { + break; + } + + if (err == 0) { + ssh_set_error(scp->session, SSH_FATAL, + "End of file while reading string"); + err = SSH_ERROR; + break; + } + + read++; + if (buffer[read - 1] == '\n') { + break; + } + } + + buffer[read] = 0; + return err; } /** @@ -544,90 +679,105 @@ int ssh_scp_read_string(ssh_scp scp, char *buffer, size_t len){ * @see ssh_scp_accept_request() * @see ssh_scp_request_get_warning() */ -int ssh_scp_pull_request(ssh_scp scp){ - char buffer[MAX_BUF_SIZE] = {0}; - char *mode=NULL; - char *p,*tmp; - uint64_t size; - char *name=NULL; - int err; - if(scp==NULL) - return SSH_ERROR; - if(scp->state != SSH_SCP_READ_INITED){ - ssh_set_error(scp->session,SSH_FATAL,"ssh_scp_pull_request called under invalid state"); - return SSH_ERROR; - } - err=ssh_scp_read_string(scp,buffer,sizeof(buffer)); - if(err==SSH_ERROR){ - if(ssh_channel_is_eof(scp->channel)){ - scp->state=SSH_SCP_TERMINATED; - return SSH_SCP_REQUEST_EOF; - } - return err; - } - p=strchr(buffer,'\n'); - if(p!=NULL) - *p='\0'; - SSH_LOG(SSH_LOG_PROTOCOL,"Received SCP request: '%s'",buffer); - switch(buffer[0]){ +int ssh_scp_pull_request(ssh_scp scp) +{ + char buffer[MAX_BUF_SIZE] = {0}; + char *mode = NULL; + char *p, *tmp; + uint64_t size; + char *name = NULL; + int rc; + + if (scp == NULL) { + return SSH_ERROR; + } + + if (scp->state != SSH_SCP_READ_INITED) { + ssh_set_error(scp->session, SSH_FATAL, + "ssh_scp_pull_request called under invalid state"); + return SSH_ERROR; + } + + rc = ssh_scp_read_string(scp, buffer, sizeof(buffer)); + if (rc == SSH_ERROR) { + if (ssh_channel_is_eof(scp->channel)) { + scp->state = SSH_SCP_TERMINATED; + return SSH_SCP_REQUEST_EOF; + } + return rc; + } + + p = strchr(buffer, '\n'); + if (p != NULL) { + *p = '\0'; + } + + SSH_LOG(SSH_LOG_PROTOCOL, "Received SCP request: '%s'", buffer); + switch(buffer[0]) { case 'C': - /* File */ + /* File */ case 'D': - /* Directory */ - p=strchr(buffer,' '); - if(p==NULL) - goto error; - *p='\0'; - p++; - //mode=strdup(&buffer[1]); - scp->request_mode=ssh_scp_integer_mode(&buffer[1]); - tmp=p; - p=strchr(p,' '); - if(p==NULL) - goto error; - *p=0; - size = strtoull(tmp,NULL,10); - p++; - name=strdup(p); - SAFE_FREE(scp->request_name); - scp->request_name=name; - if(buffer[0]=='C'){ - scp->filelen=size; - scp->request_type=SSH_SCP_REQUEST_NEWFILE; - } else { - scp->filelen='0'; - scp->request_type=SSH_SCP_REQUEST_NEWDIR; - } - scp->state=SSH_SCP_READ_REQUESTED; - scp->processed = 0; - return scp->request_type; - break; + /* Directory */ + p = strchr(buffer, ' '); + if (p == NULL) { + goto error; + } + *p = '\0'; + p++; + //mode = strdup(&buffer[1]); + scp->request_mode = ssh_scp_integer_mode(&buffer[1]); + tmp = p; + p = strchr(p, ' '); + if (p == NULL) { + goto error; + } + *p = 0; + size = strtoull(tmp, NULL, 10); + p++; + name = strdup(p); + SAFE_FREE(scp->request_name); + scp->request_name = name; + if (buffer[0] == 'C') { + scp->filelen = size; + scp->request_type = SSH_SCP_REQUEST_NEWFILE; + } else { + scp->filelen = '0'; + scp->request_type = SSH_SCP_REQUEST_NEWDIR; + } + scp->state = SSH_SCP_READ_REQUESTED; + scp->processed = 0; + return scp->request_type; + break; case 'E': - scp->request_type=SSH_SCP_REQUEST_ENDDIR; - ssh_channel_write(scp->channel,"",1); - return scp->request_type; + scp->request_type = SSH_SCP_REQUEST_ENDDIR; + ssh_channel_write(scp->channel, "", 1); + return scp->request_type; case 0x1: - ssh_set_error(scp->session,SSH_REQUEST_DENIED,"SCP: Warning: %s",&buffer[1]); - scp->request_type=SSH_SCP_REQUEST_WARNING; - SAFE_FREE(scp->warning); - scp->warning=strdup(&buffer[1]); - return scp->request_type; + ssh_set_error(scp->session, SSH_REQUEST_DENIED, + "SCP: Warning: %s", &buffer[1]); + scp->request_type = SSH_SCP_REQUEST_WARNING; + SAFE_FREE(scp->warning); + scp->warning = strdup(&buffer[1]); + return scp->request_type; case 0x2: - ssh_set_error(scp->session,SSH_FATAL,"SCP: Error: %s",&buffer[1]); - return SSH_ERROR; + ssh_set_error(scp->session, SSH_FATAL, + "SCP: Error: %s", &buffer[1]); + return SSH_ERROR; case 'T': - /* Timestamp */ + /* Timestamp */ default: - ssh_set_error(scp->session,SSH_FATAL,"Unhandled message: (%d)%s",buffer[0],buffer); - return SSH_ERROR; - } - - /* a parsing error occured */ - error: - SAFE_FREE(name); - SAFE_FREE(mode); - ssh_set_error(scp->session,SSH_FATAL,"Parsing error while parsing message: %s",buffer); - return SSH_ERROR; + ssh_set_error(scp->session, SSH_FATAL, + "Unhandled message: (%d)%s", buffer[0], buffer); + return SSH_ERROR; + } + + /* a parsing error occured */ +error: + SAFE_FREE(name); + SAFE_FREE(mode); + ssh_set_error(scp->session, SSH_FATAL, + "Parsing error while parsing message: %s", buffer); + return SSH_ERROR; } /** @@ -641,24 +791,31 @@ int ssh_scp_pull_request(ssh_scp scp){ * @returns SSH_OK if the message was sent, SSH_ERROR if the sending * the message failed, or sending it in a bad state. */ -int ssh_scp_deny_request(ssh_scp scp, const char *reason){ - char buffer[MAX_BUF_SIZE]; - int err; - if(scp==NULL) - return SSH_ERROR; - if(scp->state != SSH_SCP_READ_REQUESTED){ - ssh_set_error(scp->session,SSH_FATAL,"ssh_scp_deny_request called under invalid state"); - return SSH_ERROR; - } - snprintf(buffer,sizeof(buffer),"%c%s\n",2,reason); - err=ssh_channel_write(scp->channel,buffer,strlen(buffer)); - if(err==SSH_ERROR) { - return SSH_ERROR; - } - else { - scp->state=SSH_SCP_READ_INITED; - return SSH_OK; - } +int ssh_scp_deny_request(ssh_scp scp, const char *reason) +{ + char buffer[MAX_BUF_SIZE] = {0}; + int rc; + + if (scp == NULL) { + return SSH_ERROR; + } + + if (scp->state != SSH_SCP_READ_REQUESTED) { + ssh_set_error(scp->session, SSH_FATAL, + "ssh_scp_deny_request called under invalid state"); + return SSH_ERROR; + } + + snprintf(buffer, sizeof(buffer), "%c%s\n", 2, reason); + rc = ssh_channel_write(scp->channel, buffer, strlen(buffer)); + if (rc == SSH_ERROR) { + return SSH_ERROR; + } + + else { + scp->state = SSH_SCP_READ_INITED; + return SSH_OK; + } } /** @@ -670,24 +827,32 @@ int ssh_scp_deny_request(ssh_scp scp, const char *reason){ * @returns SSH_OK if the message was sent, SSH_ERROR if sending the * message failed, or sending it in a bad state. */ -int ssh_scp_accept_request(ssh_scp scp){ - char buffer[]={0x00}; - int err; - if(scp==NULL) - return SSH_ERROR; - if(scp->state != SSH_SCP_READ_REQUESTED){ - ssh_set_error(scp->session,SSH_FATAL,"ssh_scp_deny_request called under invalid state"); - return SSH_ERROR; - } - err=ssh_channel_write(scp->channel,buffer,1); - if(err==SSH_ERROR) { - return SSH_ERROR; - } - if(scp->request_type==SSH_SCP_REQUEST_NEWFILE) - scp->state=SSH_SCP_READ_READING; - else - scp->state=SSH_SCP_READ_INITED; - return SSH_OK; +int ssh_scp_accept_request(ssh_scp scp) +{ + char buffer[] = {0x00}; + int rc; + if (scp == NULL) { + return SSH_ERROR; + } + + if (scp->state != SSH_SCP_READ_REQUESTED) { + ssh_set_error(scp->session, SSH_FATAL, + "ssh_scp_deny_request called under invalid state"); + return SSH_ERROR; + } + + rc = ssh_channel_write(scp->channel, buffer, 1); + if (rc == SSH_ERROR) { + return SSH_ERROR; + } + + if (scp->request_type == SSH_SCP_REQUEST_NEWFILE) { + scp->state = SSH_SCP_READ_READING; + } else { + scp->state = SSH_SCP_READ_INITED; + } + + return SSH_OK; } /** @brief Read from a remote scp file @@ -700,48 +865,64 @@ int ssh_scp_accept_request(ssh_scp scp){ * @returns The nNumber of bytes read, SSH_ERROR if an error occured * while reading. */ -int ssh_scp_read(ssh_scp scp, void *buffer, size_t size){ - int r; - int code; - if(scp==NULL) - return SSH_ERROR; - if(scp->state == SSH_SCP_READ_REQUESTED && scp->request_type == SSH_SCP_REQUEST_NEWFILE){ - r=ssh_scp_accept_request(scp); - if(r==SSH_ERROR) - return r; - } - if(scp->state != SSH_SCP_READ_READING){ - ssh_set_error(scp->session,SSH_FATAL,"ssh_scp_read called under invalid state"); - return SSH_ERROR; - } - if(scp->processed + size > scp->filelen) - size = (size_t) (scp->filelen - scp->processed); - if(size > 65536) - size=65536; /* avoid too large reads */ - r=ssh_channel_read(scp->channel,buffer,size,0); - if(r != SSH_ERROR) - scp->processed += r; - else { - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; - } - /* Check if we arrived at end of file */ - if(scp->processed == scp->filelen) { - scp->processed=scp->filelen=0; - ssh_channel_write(scp->channel,"",1); - code=ssh_scp_response(scp,NULL); - if(code == 0){ - scp->state=SSH_SCP_READ_INITED; - return r; - } - if(code==1){ - scp->state=SSH_SCP_READ_INITED; - return SSH_ERROR; - } - scp->state=SSH_SCP_ERROR; - return SSH_ERROR; - } - return r; +int ssh_scp_read(ssh_scp scp, void *buffer, size_t size) +{ + int rc; + int code; + + if (scp == NULL) { + return SSH_ERROR; + } + + if (scp->state == SSH_SCP_READ_REQUESTED && + scp->request_type == SSH_SCP_REQUEST_NEWFILE) + { + rc = ssh_scp_accept_request(scp); + if (rc == SSH_ERROR) { + return rc; + } + } + + if (scp->state != SSH_SCP_READ_READING) { + ssh_set_error(scp->session, SSH_FATAL, + "ssh_scp_read called under invalid state"); + return SSH_ERROR; + } + + if (scp->processed + size > scp->filelen) { + size = (size_t) (scp->filelen - scp->processed); + } + + if (size > 65536) { + size = 65536; /* avoid too large reads */ + } + + rc = ssh_channel_read(scp->channel, buffer, size, 0); + if (rc != SSH_ERROR) { + scp->processed += rc; + } else { + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + /* Check if we arrived at end of file */ + if (scp->processed == scp->filelen) { + scp->processed = scp->filelen = 0; + ssh_channel_write(scp->channel, "", 1); + code = ssh_scp_response(scp, NULL); + if (code == 0) { + scp->state = SSH_SCP_READ_INITED; + return rc; + } + if (code == 1) { + scp->state = SSH_SCP_READ_INITED; + return SSH_ERROR; + } + scp->state = SSH_SCP_ERROR; + return SSH_ERROR; + } + + return rc; } /** @@ -751,10 +932,13 @@ int ssh_scp_read(ssh_scp scp, void *buffer, size_t size){ * @returns The file name, NULL on error. The string should not be * freed. */ -const char *ssh_scp_request_get_filename(ssh_scp scp){ - if(scp==NULL) - return NULL; - return scp->request_name; +const char *ssh_scp_request_get_filename(ssh_scp scp) +{ + if (scp == NULL) { + return NULL; + } + + return scp->request_name; } /** @@ -763,10 +947,13 @@ const char *ssh_scp_request_get_filename(ssh_scp scp){ * * @returns The UNIX permission, e.g 0644, -1 on error. */ -int ssh_scp_request_get_permissions(ssh_scp scp){ - if(scp==NULL) - return -1; - return scp->request_mode; +int ssh_scp_request_get_permissions(ssh_scp scp) +{ + if (scp == NULL) { + return -1; + } + + return scp->request_mode; } /** @brief Get the size of the file being pushed from the other party. @@ -776,20 +963,24 @@ int ssh_scp_request_get_permissions(ssh_scp scp){ * be truncated. * @see ssh_scp_request_get_size64() */ -size_t ssh_scp_request_get_size(ssh_scp scp){ - if(scp==NULL) - return 0; - return (size_t)scp->filelen; +size_t ssh_scp_request_get_size(ssh_scp scp) +{ + if (scp == NULL) { + return 0; + } + return (size_t)scp->filelen; } /** @brief Get the size of the file being pushed from the other party. * * @returns The numeric size of the file being read. */ -uint64_t ssh_scp_request_get_size64(ssh_scp scp){ - if(scp==NULL) - return 0; - return scp->filelen; +uint64_t ssh_scp_request_get_size64(ssh_scp scp) +{ + if (scp == NULL) { + return 0; + } + return scp->filelen; } /** @@ -799,9 +990,10 @@ uint64_t ssh_scp_request_get_size64(ssh_scp scp){ * * @returns An integer value, e.g. 420 for "0644". */ -int ssh_scp_integer_mode(const char *mode){ - int value=strtoul(mode,NULL,8) & 0xffff; - return value; +int ssh_scp_integer_mode(const char *mode) +{ + int value = strtoul(mode, NULL, 8) & 0xffff; + return value; } /** @@ -812,10 +1004,11 @@ int ssh_scp_integer_mode(const char *mode){ * @returns A pointer to a malloc'ed string containing the scp mode, * e.g. "0644". */ -char *ssh_scp_string_mode(int mode){ - char buffer[16]; - snprintf(buffer,sizeof(buffer),"%.4o",mode); - return strdup(buffer); +char *ssh_scp_string_mode(int mode) +{ + char buffer[16] = {0}; + snprintf(buffer, sizeof(buffer), "%.4o", mode); + return strdup(buffer); } /** @@ -826,10 +1019,13 @@ char *ssh_scp_string_mode(int mode){ * @returns A warning string, or NULL on error. The string should * not be freed. */ -const char *ssh_scp_request_get_warning(ssh_scp scp){ - if(scp==NULL) - return NULL; - return scp->warning; +const char *ssh_scp_request_get_warning(ssh_scp scp) +{ + if (scp == NULL) { + return NULL; + } + + return scp->warning; } /** @} */ -- 2.21.0 From 0c85d47f0694fda268c7bb1f170ecfc29942f83c Mon Sep 17 00:00:00 2001 From: Anderson Toshiyuki Sasaki <ansasaki@redhat.com> Date: Fri, 25 Oct 2019 13:24:28 +0200 Subject: [PATCH 2/4] CVE-2019-14889: scp: Log SCP warnings received from the server Fixes T181 Previously, warnings received from the server were ignored. With this change the warning message sent by the server will be logged. Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org> (cherry picked from commit 8d57f33bd919970d928fd162ccdaf81d270686e4) (cherry picked from commit 77b062de91c0a93a17bf45b92dc6da5756e18bc7) --- src/scp.c | 75 ++++++++----------------------------------------------- 1 file changed, 11 insertions(+), 64 deletions(-) diff --git a/src/scp.c b/src/scp.c index 5de0e6ff..166f3d2f 100644 --- a/src/scp.c +++ b/src/scp.c @@ -113,7 +113,6 @@ int ssh_scp_init(ssh_scp scp) { int rc; char execbuffer[1024] = {0}; - uint8_t code; if (scp == NULL) { return SSH_ERROR; @@ -157,19 +156,8 @@ int ssh_scp_init(ssh_scp scp) } if (scp->mode == SSH_SCP_WRITE) { - rc = ssh_channel_read(scp->channel, &code, 1, 0); - if (rc <= 0) { - ssh_set_error(scp->session, SSH_FATAL, - "Error reading status code: %s", - ssh_get_error(scp->session)); - scp->state = SSH_SCP_ERROR; - return SSH_ERROR; - } - - if (code != 0) { - ssh_set_error(scp->session, SSH_FATAL, - "scp status code %ud not valid", code); - scp->state = SSH_SCP_ERROR; + rc = ssh_scp_response(scp, NULL); + if (rc != 0) { return SSH_ERROR; } } else { @@ -277,7 +265,6 @@ int ssh_scp_push_directory(ssh_scp scp, const char *dirname, int mode) { char buffer[1024] = {0}; int rc; - uint8_t code; char *dir = NULL; char *perms = NULL; @@ -303,19 +290,8 @@ int ssh_scp_push_directory(ssh_scp scp, const char *dirname, int mode) return SSH_ERROR; } - rc = ssh_channel_read(scp->channel, &code, 1, 0); - if (rc <= 0) { - ssh_set_error(scp->session, SSH_FATAL, - "Error reading status code: %s", - ssh_get_error(scp->session)); - scp->state = SSH_SCP_ERROR; - return SSH_ERROR; - } - - if (code != 0) { - ssh_set_error(scp->session, SSH_FATAL, "scp status code %ud not valid", - code); - scp->state = SSH_SCP_ERROR; + rc = ssh_scp_response(scp, NULL); + if (rc != 0) { return SSH_ERROR; } @@ -334,7 +310,6 @@ int ssh_scp_leave_directory(ssh_scp scp) { char buffer[] = "E\n"; int rc; - uint8_t code; if (scp == NULL) { return SSH_ERROR; @@ -352,18 +327,8 @@ int ssh_scp_leave_directory(ssh_scp scp) return SSH_ERROR; } - rc = ssh_channel_read(scp->channel, &code, 1, 0); - if (rc <= 0) { - ssh_set_error(scp->session, SSH_FATAL, "Error reading status code: %s", - ssh_get_error(scp->session)); - scp->state = SSH_SCP_ERROR; - return SSH_ERROR; - } - - if (code != 0) { - ssh_set_error(scp->session, SSH_FATAL, "scp status code %ud not valid", - code); - scp->state = SSH_SCP_ERROR; + rc = ssh_scp_response(scp, NULL); + if (rc != 0) { return SSH_ERROR; } @@ -395,7 +360,6 @@ int ssh_scp_push_file64(ssh_scp scp, const char *filename, uint64_t size, int rc; char *file = NULL; char *perms = NULL; - uint8_t code; if (scp == NULL) { return SSH_ERROR; @@ -422,19 +386,8 @@ int ssh_scp_push_file64(ssh_scp scp, const char *filename, uint64_t size, return SSH_ERROR; } - rc = ssh_channel_read(scp->channel, &code, 1, 0); - if (rc <= 0) { - ssh_set_error(scp->session, SSH_FATAL, - "Error reading status code: %s", - ssh_get_error(scp->session)); - scp->state = SSH_SCP_ERROR; - return SSH_ERROR; - } - - if (code != 0) { - ssh_set_error(scp->session, SSH_FATAL, - "scp status code %ud not valid", code); - scp->state = SSH_SCP_ERROR; + rc = ssh_scp_response(scp, NULL); + if (rc != 0) { return SSH_ERROR; } @@ -498,7 +451,7 @@ int ssh_scp_response(ssh_scp scp, char **response) if (code > 2) { ssh_set_error(scp->session, SSH_FATAL, - "SCP: invalid status code %ud received", code); + "SCP: invalid status code %u received", code); scp->state = SSH_SCP_ERROR; return SSH_ERROR; } @@ -585,14 +538,8 @@ int ssh_scp_write(ssh_scp scp, const void *buffer, size_t len) * and handle */ rc = ssh_channel_poll(scp->channel, 0); if (rc > 0) { - rc = ssh_channel_read(scp->channel, &code, 1, 0); - if (rc == SSH_ERROR) { - return SSH_ERROR; - } - - if (code == 1 || code == 2) { - ssh_set_error(scp->session, SSH_REQUEST_DENIED, - "SCP: Error: status code %i received", code); + rc = ssh_scp_response(scp, NULL); + if (rc != 0) { return SSH_ERROR; } } -- 2.21.0 From 40a8ef04d9984c7c54340fc7d1f31942b1e4ba4b Mon Sep 17 00:00:00 2001 From: Anderson Toshiyuki Sasaki <ansasaki@redhat.com> Date: Tue, 22 Oct 2019 16:08:24 +0200 Subject: [PATCH 3/4] CVE-2019-14889: misc: Add function to quote file names The added function quote file names strings to be used in a shell. Special cases are treated for the charactes '\'' and '!'. Fixes T181 Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com> Reviewed-by: Andreas Schneider <asn@cryptomilk.org> (cherry picked from commit 93cd7cf6be66e24a0ac54c3007bd23be3d66e5ae) (cherry picked from commit e6ba1d609d9567569a461a8bb2213d697a8e3005) --- include/libssh/misc.h | 8 ++ src/misc.c | 175 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 183 insertions(+) diff --git a/include/libssh/misc.h b/include/libssh/misc.h index bc50cff8..0531d4f3 100644 --- a/include/libssh/misc.h +++ b/include/libssh/misc.h @@ -50,6 +50,12 @@ struct ssh_timestamp { long useconds; }; +enum ssh_quote_state_e { + NO_QUOTE, + SINGLE_QUOTE, + DOUBLE_QUOTE +}; + struct ssh_list *ssh_list_new(void); void ssh_list_free(struct ssh_list *list); struct ssh_iterator *ssh_list_get_iterator(const struct ssh_list *list); @@ -81,4 +87,6 @@ int ssh_timeout_update(struct ssh_timestamp *ts, int timeout); int ssh_match_group(const char *group, const char *object); +int ssh_quote_file_name(const char *file_name, char *buf, size_t buf_len); + #endif /* MISC_H_ */ diff --git a/src/misc.c b/src/misc.c index 18c9745e..bb644fa4 100644 --- a/src/misc.c +++ b/src/misc.c @@ -1108,4 +1108,179 @@ char *strndup(const char *s, size_t n) } #endif /* ! HAVE_STRNDUP */ +/** + * @internal + * + * @brief Quote file name to be used on shell. + * + * Try to put the given file name between single quotes. There are special + * cases: + * + * - When the '\'' char is found in the file name, it is double quoted + * - example: + * input: a'b + * output: 'a'"'"'b' + * - When the '!' char is found in the file name, it is replaced by an unquoted + * verbatim char "\!" + * - example: + * input: a!b + * output 'a'\!'b' + * + * @param[in] file_name File name string to be quoted before used on shell + * @param[out] buf Buffer to receive the final quoted file name. Must + * have room for the final quoted string. The maximum + * output length would be (3 * strlen(file_name) + 1) + * since in the worst case each character would be + * replaced by 3 characters, plus the terminating '\0'. + * @param[in] buf_len The size of the provided output buffer + * + * @returns SSH_ERROR on error; length of the resulting string not counting the + * string terminator '\0' + * */ +int ssh_quote_file_name(const char *file_name, char *buf, size_t buf_len) +{ + const char *src = NULL; + char *dst = NULL; + + enum ssh_quote_state_e state = NO_QUOTE; + + if (file_name == NULL || buf == NULL || buf_len == 0) { + SSH_LOG(SSH_LOG_WARNING, "Invalid parameter"); + return SSH_ERROR; + } + + if ((3 * strlen(file_name) + 1) > buf_len) { + SSH_LOG(SSH_LOG_WARNING, "Buffer too small"); + return SSH_ERROR; + } + + src = file_name; + dst = buf; + + while ((*src != '\0')) { + switch (*src) { + + /* The '\'' char is double quoted */ + + case '\'': + switch (state) { + case NO_QUOTE: + /* Start a new double quoted string. The '\'' char will be + * copied to the beginning of it at the end of the loop. */ + *dst++ = '"'; + break; + case SINGLE_QUOTE: + /* Close the current single quoted string and start a new double + * quoted string. The '\'' char will be copied to the beginning + * of it at the end of the loop. */ + *dst++ = '\''; + *dst++ = '"'; + break; + case DOUBLE_QUOTE: + /* If already in the double quoted string, keep copying the + * sequence of chars. */ + break; + default: + /* Should never be reached */ + goto error; + } + + /* When the '\'' char is found, the resulting state will be + * DOUBLE_QUOTE in any case*/ + state = DOUBLE_QUOTE; + break; + + /* The '!' char is replaced by unquoted "\!" */ + + case '!': + switch (state) { + case NO_QUOTE: + /* The '!' char is interpreted in some shells (e.g. CSH) even + * when is quoted with single quotes. Replace it with unquoted + * "\!" which is correctly interpreted as the '!' character. */ + *dst++ = '\\'; + break; + case SINGLE_QUOTE: + /* Close the current quoted string and replace '!' for unquoted + * "\!" */ + *dst++ = '\''; + *dst++ = '\\'; + break; + case DOUBLE_QUOTE: + /* Close current quoted string and replace "!" for unquoted + * "\!" */ + *dst++ = '"'; + *dst++ = '\\'; + break; + default: + /* Should never be reached */ + goto error; + } + + /* When the '!' char is found, the resulting state will be NO_QUOTE + * in any case*/ + state = NO_QUOTE; + break; + + /* Ordinary chars are single quoted */ + + default: + switch (state) { + case NO_QUOTE: + /* Start a new single quoted string */ + *dst++ = '\''; + break; + case SINGLE_QUOTE: + /* If already in the single quoted string, keep copying the + * sequence of chars. */ + break; + case DOUBLE_QUOTE: + /* Close current double quoted string and start a new single + * quoted string. */ + *dst++ = '"'; + *dst++ = '\''; + break; + default: + /* Should never be reached */ + goto error; + } + + /* When an ordinary char is found, the resulting state will be + * SINGLE_QUOTE in any case*/ + state = SINGLE_QUOTE; + break; + } + + /* Copy the current char to output */ + *dst++ = *src++; + } + + /* Close the quoted string when necessary */ + + switch (state) { + case NO_QUOTE: + /* No open string */ + break; + case SINGLE_QUOTE: + /* Close current single quoted string */ + *dst++ = '\''; + break; + case DOUBLE_QUOTE: + /* Close current double quoted string */ + *dst++ = '"'; + break; + default: + /* Should never be reached */ + goto error; + } + + /* Put the string terminator */ + *dst = '\0'; + + return dst - buf; + +error: + return SSH_ERROR; +} + /** @} */ -- 2.21.0 From 566bb87e11332b140d9e9035726b05bf20e97011 Mon Sep 17 00:00:00 2001 From: Anderson Toshiyuki Sasaki <ansasaki@redhat.com> Date: Thu, 31 Oct 2019 18:10:27 +0100 Subject: [PATCH 4/4] 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 6ff83649916c2b0820cf1d449d329eadbb1b6f7d) (cherry picked from commit 9859fbaf8706ebb84a1756e524245e9e5374786c) --- src/scp.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/src/scp.c b/src/scp.c index 166f3d2f..decab42c 100644 --- a/src/scp.c +++ b/src/scp.c @@ -29,6 +29,7 @@ #include "libssh/priv.h" #include "libssh/scp.h" +#include "libssh/misc.h" /** * @defgroup libssh_scp The SSH scp functions @@ -113,6 +114,8 @@ int ssh_scp_init(ssh_scp scp) { int rc; char execbuffer[1024] = {0}; + char *quoted_location = NULL; + size_t quoted_location_len = 0; if (scp == NULL) { return SSH_ERROR; @@ -124,33 +127,71 @@ 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' */ + quoted_location_len = (3 * strlen(scp->location)) + 1; + + 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; } -- 2.21.0
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor