[PATCH] channels: Fix segfaults when the channel data is freed

["Artyom V. Poptsov" <poptsov.artyom@xxxxxxxxx>]

Hello,

I've found that some channel procedures in libssh 0.9.3 on Ubuntu 
GNU/Linux 20.04 (and the latest versions from Git) fails with SEGFAULT 
when a parent session of a channel is freed.

I investigated the errors and found that some "channels.h" procedures 
(namely 'ssh_channel_is_open' and 'ssh_channel_is_closed') try to 
determine whether the parent session is alive by checking 
'channel->session->alive' variable.  But when a channel is freed by 
'ssh_channel_do_free' procedure, its 'session' field is set to NULL.  In 
that situation calling e.g. 'ssh_channel_is_open' on a channel will lead 
to a SEGFAULT.

The patch to fix that behavior is attached to the email.

I ran "make test" on the libssh sources with my changes and all tests 
have passed successfully.

Thanks.

--
Artyom "avp" Poptsov <poptsov.artyom@xxxxxxxxx>
Home page: https://memory-heap.org/~avp/
CADR Hackerspace co-founder: https://cadrspace.ru/
GPG: D0C2 EAC1 3310 822D 98DE  B57C E9C5 A2D9 0898 A02F

From f96c7ded5fb61589fd38056fd1431021e4f441cb Mon Sep 17 00:00:00 2001
From: "Artyom V. Poptsov" <poptsov.artyom@xxxxxxxxx>
Date: Sun, 15 Aug 2021 22:50:33 +0300
Subject: [PATCH] channels: Fix segfaults when the channel data is freed

libssh fails to properly determine whether the parent session of a
channel is alive or not, resulting in SEGFAULT errors.  The reason is
that when a channel is freed with 'ssh_channel_do_free' procedure, its
'session' field is set to NULL; then when a channel procedure tries to
check the value of 'channel->session->alive' it effectively
dereferencing a NULL pointer.

The change fixes that behavior.

Signed-off-by: Artyom V. Poptsov <poptsov.artyom@xxxxxxxxx>
---
 src/channels.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/channels.c b/src/channels.c
index 8d812477..a71b5c73 100644
--- a/src/channels.c
+++ b/src/channels.c
@@ -1613,7 +1613,9 @@ int ssh_channel_is_open(ssh_channel channel) {
     if(channel == NULL) {
         return 0;
     }
-    return (channel->state == SSH_CHANNEL_STATE_OPEN && channel->session->alive != 0);
+    return ((channel->session != NULL)
+            && (channel->state == SSH_CHANNEL_STATE_OPEN)
+            && (channel->session->alive != 0));
 }
 
 /**
@@ -1629,7 +1631,9 @@ int ssh_channel_is_closed(ssh_channel channel) {
     if(channel == NULL) {
         return SSH_ERROR;
     }
-    return (channel->state != SSH_CHANNEL_STATE_OPEN || channel->session->alive == 0);
+    return ((! channel->session)
+            || (channel->state != SSH_CHANNEL_STATE_OPEN)
+            || (channel->session->alive == 0));
 }
 
 /**
@@ -3094,7 +3098,7 @@ int ssh_channel_read_nonblocking(ssh_channel channel,
 int ssh_channel_poll(ssh_channel channel, int is_stderr){
   ssh_buffer stdbuf;
 
-  if(channel == NULL) {
+  if((channel == NULL) || (channel->session == NULL)) {
       return SSH_ERROR;
   }
 
@@ -3151,7 +3155,7 @@ int ssh_channel_poll_timeout(ssh_channel channel, int timeout, int is_stderr)
     size_t len;
     int rc;
 
-    if (channel == NULL) {
+    if ((channel == NULL) || (channel->session == NULL)) {
         return SSH_ERROR;
     }
 
@@ -3241,7 +3245,7 @@ static int ssh_channel_exit_status_termination(void *c){
  */
 int ssh_channel_get_exit_status(ssh_channel channel) {
   int rc;
-  if(channel == NULL) {
+  if((channel == NULL) || (channel->session == NULL)) {
       return SSH_ERROR;
   }
   rc = ssh_handle_packets_termination(channel->session,
-- 
2.25.1

Attachment: OpenPGP_signature
Description: OpenPGP digital signature