[PATCH] Set and propagate sftp errors

[Anderson Sasaki <ansasaki@xxxxxxxxxx>]

Hello,

I am investigating an issue in curl github [1] where some issue occurs in sftp.
After Andreas fixed an issue where any sftp error would be overwritten [2], I realized the sftp error is not always set when it fails.
Since curl (and maybe other applications) relies on sftp_get_error() to log which error occured, it is important to correctly set it.

I am not familiar with SFTP, so I am not sure if I can, in client side, set some of the errors (e.g. SSH_FX_BAD_MESSAGE).
Please review the attached patches and tell me if they make sense.

Regards,
Anderson

[1] https://github.com/curl/curl/issues/3310
[2] https://git.libssh.org/projects/libssh.git/commit/?id=3784226fd85bc1256ef927640f4d400348da038f

From a634c01c1719ee7265396643723b765bd3caa389 Mon Sep 17 00:00:00 2001
From: Anderson Toshiyuki Sasaki <ansasaki@xxxxxxxxxx>
Date: Mon, 3 Dec 2018 17:25:22 +0100
Subject: [PATCH 1/7] sftp: Set error when EOF is received in
 sftp_packet_read()

When reading a sftp packet and an EOF is received before all requested
bytes are read, set the session and sftp error codes.

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@xxxxxxxxxx>
---
 src/sftp.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/sftp.c b/src/sftp.c
index b4ddedcd..88fa8a41 100644
--- a/src/sftp.c
+++ b/src/sftp.c
@@ -396,6 +396,9 @@ sftp_packet sftp_packet_read(sftp_session sftp)
         } else if (s == 0) {
             is_eof = ssh_channel_is_eof(sftp->channel);
             if (is_eof) {
+                ssh_set_error(sftp->session, SSH_FATAL,
+                    "Received EOF while reading sftp packet size");
+                sftp_set_error(sftp, SSH_FX_EOF);
                 goto error;
             }
         } else {
@@ -416,6 +419,9 @@ sftp_packet sftp_packet_read(sftp_session sftp)
         } else if (nread == 0) {
             is_eof = ssh_channel_is_eof(sftp->channel);
             if (is_eof) {
+                ssh_set_error(sftp->session, SSH_FATAL,
+                    "Received EOF while reading sftp packet type");
+                sftp_set_error(sftp, SSH_FX_EOF);
                 goto error;
             }
         }
@@ -451,6 +457,9 @@ sftp_packet sftp_packet_read(sftp_session sftp)
             /* Retry the reading unless the remote was closed */
             is_eof = ssh_channel_is_eof(sftp->channel);
             if (is_eof) {
+                ssh_set_error(sftp->session, SSH_REQUEST_DENIED,
+                    "Received EOF while reading sftp packet");
+                sftp_set_error(sftp, SSH_FX_EOF);
                 goto error;
             }
         }
-- 
2.19.1


From 2dcbedee7f6bdb8fb98f79e59810e427f6df08bd Mon Sep 17 00:00:00 2001
From: Anderson Toshiyuki Sasaki <ansasaki@xxxxxxxxxx>
Date: Mon, 3 Dec 2018 17:27:08 +0100
Subject: [PATCH 2/7] channels: Set error state when closed channel is read

When an attempt to read a closed channel happens, set the session error
state properly.

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@xxxxxxxxxx>
---
 src/channels.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/channels.c b/src/channels.c
index 0dfa946d..1edbd139 100644
--- a/src/channels.c
+++ b/src/channels.c
@@ -2766,8 +2766,12 @@ int ssh_channel_read_timeout(ssh_channel channel,
   /*
    * If the channel is closed or in an error state, reading from it is an error
    */
-  if (session->session_state == SSH_SESSION_STATE_ERROR ||
-      channel->state == SSH_CHANNEL_STATE_CLOSED) {
+  if (session->session_state == SSH_SESSION_STATE_ERROR) {
+      return SSH_ERROR;
+  }
+  if (channel->state == SSH_CHANNEL_STATE_CLOSED) {
+      ssh_set_error(session, SSH_FATAL,
+          "Remote channel is closed.");
       return SSH_ERROR;
   }
   if (channel->remote_eof && ssh_buffer_get_len(stdbuf) == 0) {
-- 
2.19.1


From 9931d38819061cfcfed9c75fdd8eb47a80446087 Mon Sep 17 00:00:00 2001
From: Anderson Toshiyuki Sasaki <ansasaki@xxxxxxxxxx>
Date: Tue, 4 Dec 2018 15:05:32 +0100
Subject: [PATCH 3/7] sftp: Set sftp error code when fail occurs

When an operation fails in sftp subsystem, set the sftp error, so that
it can be obtained by sftp_get_error().

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@xxxxxxxxxx>
---
 src/sftp.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/src/sftp.c b/src/sftp.c
index 88fa8a41..0502e381 100644
--- a/src/sftp.c
+++ b/src/sftp.c
@@ -336,6 +336,7 @@ int sftp_packet_write(sftp_session sftp, uint8_t type, ssh_buffer payload)
     rc = ssh_buffer_prepend_data(payload, header, sizeof(header));
     if (rc < 0) {
         ssh_set_error_oom(sftp->session);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return -1;
     }
 
@@ -343,6 +344,7 @@ int sftp_packet_write(sftp_session sftp, uint8_t type, ssh_buffer payload)
                              ssh_buffer_get(payload),
                              ssh_buffer_get_len(payload));
     if (size < 0) {
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return -1;
     }
 
@@ -375,12 +377,14 @@ sftp_packet sftp_packet_read(sftp_session sftp)
         rc = ssh_buffer_reinit(packet->payload);
         if (rc != 0) {
             ssh_set_error_oom(sftp->session);
+            sftp_set_error(sftp, SSH_FX_FAILURE);
             return NULL;
         }
     } else {
         packet->payload = ssh_buffer_new();
         if (packet->payload == NULL) {
             ssh_set_error_oom(sftp->session);
+            sftp_set_error(sftp, SSH_FX_FAILURE);
             return NULL;
         }
     }
@@ -409,6 +413,7 @@ sftp_packet sftp_packet_read(sftp_session sftp)
     size = PULL_BE_U32(buffer, 0);
     if (size == 0 || size > SFTP_PACKET_SIZE_MAX) {
         ssh_set_error(sftp->session, SSH_FATAL, "Invalid sftp packet size!");
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         goto error;
     }
 
@@ -435,6 +440,7 @@ sftp_packet sftp_packet_read(sftp_session sftp)
     nread = ssh_buffer_allocate_size(packet->payload, size);
     if (nread < 0) {
         ssh_set_error_oom(sftp->session);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         goto error;
     }
     while (size > 0 && size < SFTP_PACKET_SIZE_MAX) {
@@ -451,6 +457,7 @@ sftp_packet sftp_packet_read(sftp_session sftp)
             rc = ssh_buffer_add_data(packet->payload, buffer, nread);
             if (rc != 0) {
                 ssh_set_error_oom(sftp->session);
+                sftp_set_error(sftp, SSH_FX_FAILURE);
                 goto error;
             }
         } else { /* nread == 0 */
@@ -517,12 +524,14 @@ static sftp_message sftp_get_message(sftp_packet packet)
                       SSH_FATAL,
                       "Unknown packet type %d",
                       packet->type);
+        sftp_set_error(packet->sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
     msg = calloc(1, sizeof(struct sftp_message_struct));
     if (msg == NULL) {
         ssh_set_error_oom(sftp->session);
+        sftp_set_error(packet->sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -538,6 +547,7 @@ static sftp_message sftp_get_message(sftp_packet packet)
         ssh_set_error(packet->sftp->session, SSH_FATAL,
                 "Invalid packet %d: no ID", packet->type);
         sftp_message_free(msg);
+        sftp_set_error(packet->sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -594,6 +604,7 @@ int sftp_init(sftp_session sftp) {
   buffer = ssh_buffer_new();
   if (buffer == NULL) {
     ssh_set_error_oom(sftp->session);
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return -1;
   }
 
@@ -601,6 +612,7 @@ int sftp_init(sftp_session sftp) {
   if (rc != SSH_OK) {
     ssh_set_error_oom(sftp->session);
     ssh_buffer_free(buffer);
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return -1;
   }
   if (sftp_packet_write(sftp, SSH_FXP_INIT, buffer) < 0) {
@@ -623,6 +635,7 @@ int sftp_init(sftp_session sftp) {
   /* TODO: are we sure there are 4 bytes ready? */
   rc = ssh_buffer_unpack(packet->payload, "d", &version);
   if (rc != SSH_OK){
+      sftp_set_error(sftp, SSH_FX_FAILURE);
       return -1;
   }
   SSH_LOG(SSH_LOG_RARE,
@@ -648,6 +661,7 @@ int sftp_init(sftp_session sftp) {
       ssh_set_error_oom(sftp->session);
       SAFE_FREE(ext_name);
       SAFE_FREE(ext_data);
+      sftp_set_error(sftp, SSH_FX_FAILURE);
       return -1;
     }
     tmp[count - 1] = ext_name;
@@ -658,6 +672,7 @@ int sftp_init(sftp_session sftp) {
       ssh_set_error_oom(sftp->session);
       SAFE_FREE(ext_name);
       SAFE_FREE(ext_data);
+      sftp_set_error(sftp, SSH_FX_FAILURE);
       return -1;
     }
     tmp[count - 1] = ext_data;
@@ -743,6 +758,7 @@ static sftp_request_queue request_queue_new(sftp_message msg) {
   queue = calloc(1, sizeof(struct sftp_request_queue_struct));
   if (queue == NULL) {
     ssh_set_error_oom(msg->sftp->session);
+    sftp_set_error(msg->sftp, SSH_FX_FAILURE);
     return NULL;
   }
 
@@ -845,6 +861,7 @@ static sftp_status_message parse_status_msg(sftp_message msg){
   status = calloc(1, sizeof(struct sftp_status_message_struct));
   if (status == NULL) {
     ssh_set_error_oom(msg->sftp->session);
+    sftp_set_error(msg->sftp, SSH_FX_FAILURE);
     return NULL;
   }
 
@@ -855,6 +872,7 @@ static sftp_status_message parse_status_msg(sftp_message msg){
     SAFE_FREE(status);
     ssh_set_error(msg->sftp->session, SSH_FATAL,
         "Invalid SSH_FXP_STATUS message");
+    sftp_set_error(msg->sftp, SSH_FX_FAILURE);
     return NULL;
   }
   rc = ssh_buffer_unpack(msg->payload, "ss",
@@ -866,6 +884,7 @@ static sftp_status_message parse_status_msg(sftp_message msg){
       SAFE_FREE(status);
       ssh_set_error(msg->sftp->session, SSH_FATAL,
         "Invalid SSH_FXP_STATUS message");
+      sftp_set_error(msg->sftp, SSH_FX_FAILURE);
       return NULL;
   }
   if (status->errormsg == NULL)
@@ -874,6 +893,7 @@ static sftp_status_message parse_status_msg(sftp_message msg){
     status->langmsg = strdup("");
   if (status->errormsg == NULL || status->langmsg == NULL) {
     ssh_set_error_oom(msg->sftp->session);
+    sftp_set_error(msg->sftp, SSH_FX_FAILURE);
     status_msg_free(status);
     return NULL;
   }
@@ -903,6 +923,7 @@ static sftp_file parse_handle_msg(sftp_message msg){
   file = calloc(1, sizeof(struct sftp_file_struct));
   if (file == NULL) {
     ssh_set_error_oom(msg->sftp->session);
+    sftp_set_error(msg->sftp, SSH_FX_FAILURE);
     return NULL;
   }
 
@@ -911,6 +932,7 @@ static sftp_file parse_handle_msg(sftp_message msg){
     ssh_set_error(msg->sftp->session, SSH_FATAL,
         "Invalid SSH_FXP_HANDLE message");
     SAFE_FREE(file);
+    sftp_set_error(msg->sftp, SSH_FX_FAILURE);
     return NULL;
   }
 
@@ -935,6 +957,7 @@ sftp_dir sftp_opendir(sftp_session sftp, const char *path)
     payload = ssh_buffer_new();
     if (payload == NULL) {
         ssh_set_error_oom(sftp->session);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -947,6 +970,7 @@ sftp_dir sftp_opendir(sftp_session sftp, const char *path)
     if (rc != SSH_OK) {
         ssh_set_error_oom(sftp->session);
         ssh_buffer_free(payload);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -1026,6 +1050,7 @@ static sftp_attributes sftp_parse_attr_4(sftp_session sftp, ssh_buffer buf,
   attr = calloc(1, sizeof(struct sftp_attributes_struct));
   if (attr == NULL) {
     ssh_set_error_oom(sftp->session);
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return NULL;
   }
 
@@ -1243,6 +1268,7 @@ static sftp_attributes sftp_parse_attr_3(sftp_session sftp, ssh_buffer buf,
     attr = calloc(1, sizeof(struct sftp_attributes_struct));
     if (attr == NULL) {
         ssh_set_error_oom(sftp->session);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -1372,6 +1398,7 @@ static sftp_attributes sftp_parse_attr_3(sftp_session sftp, ssh_buffer buf,
     SAFE_FREE(attr->group);
     SAFE_FREE(attr);
     ssh_set_error(sftp->session, SSH_FATAL, "Invalid ATTR structure");
+    sftp_set_error(sftp, SSH_FX_FAILURE);
 
     return NULL;
 }
@@ -1460,6 +1487,7 @@ sftp_attributes sftp_readdir(sftp_session sftp, sftp_dir dir)
         payload = ssh_buffer_new();
         if (payload == NULL) {
             ssh_set_error_oom(sftp->session);
+            sftp_set_error(sftp, SSH_FX_FAILURE);
             return NULL;
         }
 
@@ -1471,6 +1499,7 @@ sftp_attributes sftp_readdir(sftp_session sftp, sftp_dir dir)
                              dir->handle);
         if (rc != 0) {
             ssh_set_error_oom(sftp->session);
+            sftp_set_error(sftp, SSH_FX_FAILURE);
             ssh_buffer_free(payload);
             return NULL;
         }
@@ -1590,6 +1619,7 @@ static int sftp_handle_close(sftp_session sftp, ssh_string handle)
     buffer = ssh_buffer_new();
     if (buffer == NULL) {
         ssh_set_error_oom(sftp->session);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return -1;
     }
 
@@ -1602,6 +1632,7 @@ static int sftp_handle_close(sftp_session sftp, ssh_string handle)
     if (rc != SSH_OK) {
         ssh_set_error_oom(sftp->session);
         ssh_buffer_free(buffer);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return -1;
     }
 
@@ -1732,6 +1763,7 @@ sftp_file sftp_open(sftp_session sftp,
     if (rc != SSH_OK) {
         ssh_set_error_oom(sftp->session);
         ssh_buffer_free(buffer);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -1739,6 +1771,7 @@ sftp_file sftp_open(sftp_session sftp,
     if (rc < 0) {
         ssh_set_error_oom(sftp->session);
         ssh_buffer_free(buffer);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -1783,6 +1816,7 @@ sftp_file sftp_open(sftp_session sftp,
                             SSH_FATAL,
                             "Cannot open in append mode. Unknown file size.");
                     sftp_close(handle);
+                    sftp_set_error(sftp, SSH_FX_FAILURE);
                     return NULL;
                 }
 
@@ -1837,6 +1871,7 @@ ssize_t sftp_read(sftp_file handle, void *buf, size_t count) {
   if (rc != SSH_OK){
     ssh_set_error_oom(sftp->session);
     ssh_buffer_free(buffer);
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return -1;
   }
   if (sftp_packet_write(handle->sftp, SSH_FXP_READ, buffer) < 0) {
@@ -1921,6 +1956,7 @@ int sftp_async_read_begin(sftp_file file, uint32_t len){
   buffer = ssh_buffer_new();
   if (buffer == NULL) {
     ssh_set_error_oom(sftp->session);
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return -1;
   }
 
@@ -1935,6 +1971,7 @@ int sftp_async_read_begin(sftp_file file, uint32_t len){
   if (rc != SSH_OK) {
     ssh_set_error_oom(sftp->session);
     ssh_buffer_free(buffer);
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return -1;
   }
   if (sftp_packet_write(sftp, SSH_FXP_READ, buffer) < 0) {
@@ -2044,6 +2081,7 @@ ssize_t sftp_write(sftp_file file, const void *buf, size_t count) {
   buffer = ssh_buffer_new();
   if (buffer == NULL) {
     ssh_set_error_oom(sftp->session);
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return -1;
   }
 
@@ -2059,6 +2097,7 @@ ssize_t sftp_write(sftp_file file, const void *buf, size_t count) {
   if (rc != SSH_OK){
     ssh_set_error_oom(sftp->session);
     ssh_buffer_free(buffer);
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return -1;
   }
   packetlen=ssh_buffer_get_len(buffer);
@@ -2159,6 +2198,7 @@ int sftp_unlink(sftp_session sftp, const char *file) {
   buffer = ssh_buffer_new();
   if (buffer == NULL) {
     ssh_set_error_oom(sftp->session);
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return -1;
   }
 
@@ -2171,6 +2211,7 @@ int sftp_unlink(sftp_session sftp, const char *file) {
   if (rc != SSH_OK) {
     ssh_set_error_oom(sftp->session);
     ssh_buffer_free(buffer);
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return -1;
   }
 
@@ -2231,6 +2272,7 @@ int sftp_rmdir(sftp_session sftp, const char *directory) {
   buffer = ssh_buffer_new();
   if (buffer == NULL) {
     ssh_set_error_oom(sftp->session);
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return -1;
   }
 
@@ -2243,6 +2285,7 @@ int sftp_rmdir(sftp_session sftp, const char *directory) {
   if (rc != SSH_OK) {
     ssh_set_error_oom(sftp->session);
     ssh_buffer_free(buffer);
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return -1;
   }
   if (sftp_packet_write(sftp, SSH_FXP_RMDIR, buffer) < 0) {
@@ -2302,6 +2345,7 @@ int sftp_mkdir(sftp_session sftp, const char *directory, mode_t mode)
     buffer = ssh_buffer_new();
     if (buffer == NULL) {
         ssh_set_error_oom(sftp->session);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return -1;
     }
 
@@ -2318,6 +2362,7 @@ int sftp_mkdir(sftp_session sftp, const char *directory, mode_t mode)
     if (rc != SSH_OK) {
         ssh_set_error_oom(sftp->session);
         ssh_buffer_free(buffer);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return -1;
     }
 
@@ -2325,6 +2370,7 @@ int sftp_mkdir(sftp_session sftp, const char *directory, mode_t mode)
     if (rc < 0) {
         ssh_set_error_oom(sftp->session);
         ssh_buffer_free(buffer);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return -1;
     }
 
@@ -2398,6 +2444,7 @@ int sftp_rename(sftp_session sftp, const char *original, const char *newname) {
   buffer = ssh_buffer_new();
   if (buffer == NULL) {
     ssh_set_error_oom(sftp->session);
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return -1;
   }
 
@@ -2411,6 +2458,7 @@ int sftp_rename(sftp_session sftp, const char *original, const char *newname) {
   if (rc != SSH_OK) {
     ssh_set_error_oom(sftp->session);
     ssh_buffer_free(buffer);
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return -1;
   }
 
@@ -2479,6 +2527,7 @@ int sftp_setstat(sftp_session sftp, const char *file, sftp_attributes attr)
     buffer = ssh_buffer_new();
     if (buffer == NULL) {
         ssh_set_error_oom(sftp->session);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return -1;
     }
 
@@ -2491,6 +2540,7 @@ int sftp_setstat(sftp_session sftp, const char *file, sftp_attributes attr)
     if (rc != SSH_OK) {
         ssh_set_error_oom(sftp->session);
         ssh_buffer_free(buffer);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return -1;
     }
 
@@ -2498,6 +2548,7 @@ int sftp_setstat(sftp_session sftp, const char *file, sftp_attributes attr)
     if (rc != 0) {
         ssh_set_error_oom(sftp->session);
         ssh_buffer_free(buffer);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return -1;
     }
 
@@ -2598,12 +2649,14 @@ int sftp_symlink(sftp_session sftp, const char *target, const char *dest) {
     return -1;
   if (target == NULL || dest == NULL) {
     ssh_set_error_invalid(sftp->session);
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return -1;
   }
 
   buffer = ssh_buffer_new();
   if (buffer == NULL) {
     ssh_set_error_oom(sftp->session);
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return -1;
   }
 
@@ -2626,6 +2679,7 @@ int sftp_symlink(sftp_session sftp, const char *target, const char *dest) {
   if (rc != SSH_OK){
       ssh_set_error_oom(sftp->session);
       ssh_buffer_free(buffer);
+      sftp_set_error(sftp, SSH_FX_FAILURE);
       return -1;
   }
 
@@ -2688,15 +2742,18 @@ char *sftp_readlink(sftp_session sftp, const char *path)
 
     if (path == NULL) {
         ssh_set_error_invalid(sftp);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
     if (sftp->version < 3){
         ssh_set_error(sftp,SSH_REQUEST_DENIED,"sftp version %d does not support sftp_readlink",sftp->version);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
     buffer = ssh_buffer_new();
     if (buffer == NULL) {
         ssh_set_error_oom(sftp->session);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -2709,6 +2766,7 @@ char *sftp_readlink(sftp_session sftp, const char *path)
     if (rc < 0) {
         ssh_set_error_oom(sftp->session);
         ssh_buffer_free(buffer);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -2738,6 +2796,7 @@ char *sftp_readlink(sftp_session sftp, const char *path)
             ssh_set_error(sftp->session,
                           SSH_ERROR,
                           "Failed to retrieve link");
+            sftp_set_error(sftp, SSH_FX_FAILURE);
             return NULL;
         }
 
@@ -2748,6 +2807,7 @@ char *sftp_readlink(sftp_session sftp, const char *path)
         if (status == NULL) {
             return NULL;
         }
+        sftp_set_error(sftp, status->status);
         ssh_set_error(sftp->session, SSH_REQUEST_DENIED,
                 "SFTP server: %s", status->errormsg);
         status_msg_free(status);
@@ -2767,6 +2827,7 @@ static sftp_statvfs_t sftp_parse_statvfs(sftp_session sftp, ssh_buffer buf) {
   statvfs = calloc(1, sizeof(struct sftp_statvfs_struct));
   if (statvfs == NULL) {
     ssh_set_error_oom(sftp->session);
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return NULL;
   }
 
@@ -2786,6 +2847,7 @@ static sftp_statvfs_t sftp_parse_statvfs(sftp_session sftp, ssh_buffer buf) {
   if (rc != SSH_OK) {
     SAFE_FREE(statvfs);
     ssh_set_error(sftp->session, SSH_FATAL, "Invalid statvfs structure");
+    sftp_set_error(sftp, SSH_FX_FAILURE);
     return NULL;
   }
 
@@ -2804,16 +2866,19 @@ sftp_statvfs_t sftp_statvfs(sftp_session sftp, const char *path)
         return NULL;
     if (path == NULL) {
         ssh_set_error_invalid(sftp->session);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
     if (sftp->version < 3){
         ssh_set_error(sftp,SSH_REQUEST_DENIED,"sftp version %d does not support sftp_statvfs",sftp->version);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
     buffer = ssh_buffer_new();
     if (buffer == NULL) {
         ssh_set_error_oom(sftp->session);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -2827,6 +2892,7 @@ sftp_statvfs_t sftp_statvfs(sftp_session sftp, const char *path)
     if (rc != SSH_OK) {
         ssh_set_error_oom(sftp->session);
         ssh_buffer_free(buffer);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -2857,6 +2923,7 @@ sftp_statvfs_t sftp_statvfs(sftp_session sftp, const char *path)
         if (status == NULL) {
             return NULL;
         }
+        sftp_set_error(sftp, status->status);
         ssh_set_error(sftp->session, SSH_REQUEST_DENIED,
                 "SFTP server: %s", status->errormsg);
         status_msg_free(status);
@@ -2885,6 +2952,7 @@ int sftp_fsync(sftp_file file)
     buffer = ssh_buffer_new();
     if (buffer == NULL) {
         ssh_set_error_oom(sftp->session);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return -1;
     }
 
@@ -2897,6 +2965,7 @@ int sftp_fsync(sftp_file file)
                          file->handle);
     if (rc < 0) {
         ssh_set_error_oom(sftp->session);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         goto done;
     }
 
@@ -2982,6 +3051,7 @@ sftp_statvfs_t sftp_fstatvfs(sftp_file file)
     buffer = ssh_buffer_new();
     if (buffer == NULL) {
         ssh_set_error_oom(sftp->session);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -2995,6 +3065,7 @@ sftp_statvfs_t sftp_fstatvfs(sftp_file file)
     if (rc < 0) {
         ssh_set_error_oom(sftp->session);
         ssh_buffer_free(buffer);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -3025,6 +3096,7 @@ sftp_statvfs_t sftp_fstatvfs(sftp_file file)
         if (status == NULL) {
             return NULL;
         }
+        sftp_set_error(sftp, status->status);
         ssh_set_error(sftp->session, SSH_REQUEST_DENIED,
                 "SFTP server: %s", status->errormsg);
         status_msg_free(status);
@@ -3058,12 +3130,14 @@ char *sftp_canonicalize_path(sftp_session sftp, const char *path)
         return NULL;
     if (path == NULL) {
         ssh_set_error_invalid(sftp->session);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
     buffer = ssh_buffer_new();
     if (buffer == NULL) {
         ssh_set_error_oom(sftp->session);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -3076,6 +3150,7 @@ char *sftp_canonicalize_path(sftp_session sftp, const char *path)
     if (rc < 0) {
         ssh_set_error_oom(sftp->session);
         ssh_buffer_free(buffer);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -3105,6 +3180,7 @@ char *sftp_canonicalize_path(sftp_session sftp, const char *path)
             ssh_set_error(sftp->session,
                           SSH_ERROR,
                           "Failed to parse canonicalized path");
+            sftp_set_error(sftp, SSH_FX_FAILURE);
             return NULL;
         }
 
@@ -3115,6 +3191,7 @@ char *sftp_canonicalize_path(sftp_session sftp, const char *path)
         if (status == NULL) {
             return NULL;
         }
+        sftp_set_error(sftp, status->status);
         ssh_set_error(sftp->session, SSH_REQUEST_DENIED,
                 "SFTP server: %s", status->errormsg);
         status_msg_free(status);
@@ -3139,12 +3216,14 @@ static sftp_attributes sftp_xstat(sftp_session sftp,
 
     if (path == NULL) {
         ssh_set_error_invalid(sftp->session);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
     buffer = ssh_buffer_new();
     if (buffer == NULL) {
         ssh_set_error_oom(sftp->session);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -3157,6 +3236,7 @@ static sftp_attributes sftp_xstat(sftp_session sftp,
     if (rc != SSH_OK) {
         ssh_set_error_oom(sftp->session);
         ssh_buffer_free(buffer);
+        sftp_set_error(sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -3216,6 +3296,7 @@ sftp_attributes sftp_fstat(sftp_file file)
     buffer = ssh_buffer_new();
     if (buffer == NULL) {
         ssh_set_error_oom(file->sftp->session);
+        sftp_set_error(file->sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -3228,6 +3309,7 @@ sftp_attributes sftp_fstat(sftp_file file)
     if (rc != SSH_OK) {
         ssh_set_error_oom(file->sftp->session);
         ssh_buffer_free(buffer);
+        sftp_set_error(file->sftp, SSH_FX_FAILURE);
         return NULL;
     }
 
@@ -3255,6 +3337,7 @@ sftp_attributes sftp_fstat(sftp_file file)
         if (status == NULL) {
             return NULL;
         }
+        sftp_set_error(file->sftp, status->status);
         ssh_set_error(file->sftp->session, SSH_REQUEST_DENIED,
                 "SFTP server: %s", status->errormsg);
         status_msg_free(status);
-- 
2.19.1


From 043871784c17ca5fce25095840ce2333898aa465 Mon Sep 17 00:00:00 2001
From: Anderson Toshiyuki Sasaki <ansasaki@xxxxxxxxxx>
Date: Tue, 4 Dec 2018 15:14:19 +0100
Subject: [PATCH 4/7] sftp: Set sftp error when received unexpected message

Set sftp error to SSH_FX_BAD_MESSAGE if an unexpected message is
received.

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@xxxxxxxxxx>
---
 src/sftp.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/src/sftp.c b/src/sftp.c
index 0502e381..99926df1 100644
--- a/src/sftp.c
+++ b/src/sftp.c
@@ -855,6 +855,7 @@ static sftp_status_message parse_status_msg(sftp_message msg){
   if (msg->packet_type != SSH_FXP_STATUS) {
     ssh_set_error(msg->sftp->session, SSH_FATAL,
         "Not a ssh_fxp_status message passed in!");
+    sftp_set_error(msg->sftp, SSH_FX_BAD_MESSAGE);
     return NULL;
   }
 
@@ -1554,6 +1555,7 @@ sftp_attributes sftp_readdir(sftp_session sftp, sftp_dir dir)
                 ssh_set_error(sftp->session, SSH_FATAL,
                         "Unsupported message back %d", msg->packet_type);
                 sftp_message_free(msg);
+                sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
 
                 return NULL;
         }
@@ -1674,6 +1676,7 @@ static int sftp_handle_close(sftp_session sftp, ssh_string handle)
             ssh_set_error(sftp->session, SSH_FATAL,
                     "Received message %d during sftp_handle_close!", msg->packet_type);
             sftp_message_free(msg);
+            sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
     }
 
     return -1;
@@ -1827,6 +1830,7 @@ sftp_file sftp_open(sftp_session sftp,
             ssh_set_error(sftp->session, SSH_FATAL,
                     "Received message %d during open!", msg->packet_type);
             sftp_message_free(msg);
+            sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
     }
 
     return NULL;
@@ -1940,6 +1944,7 @@ ssize_t sftp_read(sftp_file handle, void *buf, size_t count) {
       ssh_set_error(sftp->session, SSH_FATAL,
           "Received message %d during read!", msg->packet_type);
       sftp_message_free(msg);
+      sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
       return -1;
   }
 
@@ -2062,6 +2067,7 @@ int sftp_async_read(sftp_file file, void *data, uint32_t size, uint32_t id){
     default:
       ssh_set_error(sftp->session,SSH_FATAL,"Received message %d during read!",msg->packet_type);
       sftp_message_free(msg);
+      sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
       return SSH_ERROR;
   }
 
@@ -2143,6 +2149,7 @@ ssize_t sftp_write(sftp_file file, const void *buf, size_t count) {
       ssh_set_error(sftp->session, SSH_FATAL,
           "Received message %d during write!", msg->packet_type);
       sftp_message_free(msg);
+      sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
       return -1;
   }
 
@@ -2256,6 +2263,7 @@ int sftp_unlink(sftp_session sftp, const char *file) {
     ssh_set_error(sftp->session,SSH_FATAL,
         "Received message %d when attempting to remove file", msg->packet_type);
     sftp_message_free(msg);
+    sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
   }
 
   return -1;
@@ -2326,6 +2334,7 @@ int sftp_rmdir(sftp_session sftp, const char *directory) {
         "Received message %d when attempting to remove directory",
         msg->packet_type);
     sftp_message_free(msg);
+    sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
   }
 
   return -1;
@@ -2428,6 +2437,7 @@ int sftp_mkdir(sftp_session sftp, const char *directory, mode_t mode)
                 "Received message %d when attempting to make directory",
                 msg->packet_type);
         sftp_message_free(msg);
+        sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
     }
 
     return -1;
@@ -2509,6 +2519,7 @@ int sftp_rename(sftp_session sftp, const char *original, const char *newname) {
         "Received message %d when attempting to rename",
         msg->packet_type);
     sftp_message_free(msg);
+    sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
   }
 
   return -1;
@@ -2592,6 +2603,7 @@ int sftp_setstat(sftp_session sftp, const char *file, sftp_attributes attr)
         ssh_set_error(sftp->session, SSH_FATAL,
                 "Received message %d when attempting to set stats", msg->packet_type);
         sftp_message_free(msg);
+        sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
     }
 
     return -1;
@@ -2723,6 +2735,7 @@ int sftp_symlink(sftp_session sftp, const char *target, const char *dest) {
     ssh_set_error(sftp->session, SSH_FATAL,
         "Received message %d when attempting to set stats", msg->packet_type);
     sftp_message_free(msg);
+    sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
   }
 
   return -1;
@@ -2815,6 +2828,7 @@ char *sftp_readlink(sftp_session sftp, const char *path)
         ssh_set_error(sftp->session, SSH_FATAL,
                 "Received message %d when attempting to set stats", msg->packet_type);
         sftp_message_free(msg);
+        sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
     }
 
     return NULL;
@@ -2931,6 +2945,7 @@ sftp_statvfs_t sftp_statvfs(sftp_session sftp, const char *path)
         ssh_set_error(sftp->session, SSH_FATAL,
                 "Received message %d when attempting to get statvfs", msg->packet_type);
         sftp_message_free(msg);
+        sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
     }
 
     return NULL;
@@ -3025,6 +3040,7 @@ int sftp_fsync(sftp_file file)
                       "Received message %d when attempting to set stats",
                       msg->packet_type);
         sftp_message_free(msg);
+        sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
     }
 
     rc = -1;
@@ -3104,6 +3120,7 @@ sftp_statvfs_t sftp_fstatvfs(sftp_file file)
         ssh_set_error(sftp->session, SSH_FATAL,
                 "Received message %d when attempting to set stats", msg->packet_type);
         sftp_message_free(msg);
+        sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
     }
 
     return NULL;
@@ -3199,6 +3216,7 @@ char *sftp_canonicalize_path(sftp_session sftp, const char *path)
         ssh_set_error(sftp->session, SSH_FATAL,
                 "Received message %d when attempting to set stats", msg->packet_type);
         sftp_message_free(msg);
+        sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
     }
 
     return NULL;
@@ -3273,6 +3291,7 @@ static sftp_attributes sftp_xstat(sftp_session sftp,
     ssh_set_error(sftp->session, SSH_FATAL,
             "Received mesg %d during stat()", msg->packet_type);
     sftp_message_free(msg);
+    sftp_set_error(sftp, SSH_FX_BAD_MESSAGE);
 
     return NULL;
 }
@@ -3347,6 +3366,7 @@ sftp_attributes sftp_fstat(sftp_file file)
     ssh_set_error(file->sftp->session, SSH_FATAL,
             "Received msg %d during fstat()", msg->packet_type);
     sftp_message_free(msg);
+    sftp_set_error(file->sftp, SSH_FX_BAD_MESSAGE);
 
     return NULL;
 }
-- 
2.19.1


From dd3ced83b672d2c93b3f5bb74402929d84d6785c Mon Sep 17 00:00:00 2001
From: Anderson Toshiyuki Sasaki <ansasaki@xxxxxxxxxx>
Date: Mon, 3 Dec 2018 18:02:33 +0100
Subject: [PATCH 5/7] sftp: Add NULL check in sftp_opendir()

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@xxxxxxxxxx>
---
 src/sftp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/sftp.c b/src/sftp.c
index 99926df1..94dbcb7f 100644
--- a/src/sftp.c
+++ b/src/sftp.c
@@ -955,6 +955,10 @@ sftp_dir sftp_opendir(sftp_session sftp, const char *path)
     uint32_t id;
     int rc;
 
+    if (sftp == NULL) {
+        return NULL;
+    }
+
     payload = ssh_buffer_new();
     if (payload == NULL) {
         ssh_set_error_oom(sftp->session);
-- 
2.19.1


From fe5df71089878c7547e9e309b2be8f87dec66e61 Mon Sep 17 00:00:00 2001
From: Anderson Toshiyuki Sasaki <ansasaki@xxxxxxxxxx>
Date: Tue, 4 Dec 2018 15:16:41 +0100
Subject: [PATCH 6/7] sftp: Add NULL check in sftp_xstat()

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@xxxxxxxxxx>
---
 src/sftp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/sftp.c b/src/sftp.c
index 94dbcb7f..7331d80d 100644
--- a/src/sftp.c
+++ b/src/sftp.c
@@ -3236,6 +3236,10 @@ static sftp_attributes sftp_xstat(sftp_session sftp,
     uint32_t id;
     int rc;
 
+    if (sftp == NULL) {
+        return NULL;
+    }
+
     if (path == NULL) {
         ssh_set_error_invalid(sftp->session);
         sftp_set_error(sftp, SSH_FX_FAILURE);
-- 
2.19.1


From bc93036ae2564d5c84187b01084f697f37afe884 Mon Sep 17 00:00:00 2001
From: Anderson Toshiyuki Sasaki <ansasaki@xxxxxxxxxx>
Date: Tue, 4 Dec 2018 15:17:06 +0100
Subject: [PATCH 7/7] sftp: Add NULL check in sftp_fstat()

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@xxxxxxxxxx>
---
 src/sftp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/sftp.c b/src/sftp.c
index 7331d80d..03eb2d11 100644
--- a/src/sftp.c
+++ b/src/sftp.c
@@ -3320,6 +3320,10 @@ sftp_attributes sftp_fstat(sftp_file file)
     uint32_t id;
     int rc;
 
+    if (file == NULL) {
+        return NULL;
+    }
+
     buffer = ssh_buffer_new();
     if (buffer == NULL) {
         ssh_set_error_oom(file->sftp->session);
-- 
2.19.1