Tighten up pointer handling after ssh_pkt_getstring.

ssh_pkt_getstring can return (NULL,0) if the input packet is too short
to contain a valid string.

In quite a few places we were passing the returned pointer,length pair
to a printf function with "%.*s" type format, which seems in practice
to have not been dereferencing the pointer but the C standard doesn't
actually guarantee that. In one place we were doing the same job by
hand with memcpy, and apparently that _can_ dereference the pointer in
practice (so a server could have caused a NULL-dereference crash by
sending an appropriately malformed "x11" type channel open request).
And also I spotted a logging call in the "forwarded-tcpip" channel
open handler which had forgotten the field width completely, so it was
erroneously relying on the string happening to be NUL-terminated in
the received packet.

I've tightened all of this up in general by normalising (NULL,0) to
("",0) before calling printf("%.*s"), and replacing the two even more
broken cases with the corrected version of that same idiom.

diff --git a/misc.h b/misc.h
index e53f89299cd70b15b4668dfcff4b29eed27496ef..db7f9143c58b377ffa4c44f9f647a8b3a8083097 100644 (file)
--- a/misc.h
+++ b/misc.h
@@ -169,4 +169,9 @@ void debug_memdump(void *buf, int len, int L);
   (cp)[0] = (unsigned char)((value) >> 8), \
   (cp)[1] = (unsigned char)(value) )
 
+/* Replace NULL with the empty string, permitting an idiom in which we
+ * get a string (pointer,length) pair that might be NULL,0 and can
+ * then safely say things like printf("%.*s", length, NULLTOEMPTY(ptr)) */
+#define NULLTOEMPTY(s) ((s)?(s):"")
+
 #endif

diff --git a/ssh.c b/ssh.c
index 1077209ae8e1fee800c0c0258db04da3b2742934..e1e94d78ac2e80def91f6b33ea9ba21c1bcf0f6d 100644 (file)
--- a/ssh.c
+++ b/ssh.c
@@ -5419,7 +5419,7 @@ static void ssh1_msg_port_open(Ssh ssh, struct Packet *pktin)
     ssh_pkt_getstring(pktin, &host, &hostsize);
     port = ssh_pkt_getuint32(pktin);
 
-    pf.dhost = dupprintf("%.*s", hostsize, host);
+    pf.dhost = dupprintf("%.*s", hostsize, NULLTOEMPTY(host));
     pf.dport = port;
     pfp = find234(ssh->rportfwds, &pf, NULL);
 
@@ -5902,7 +5902,7 @@ static void ssh1_msg_debug(Ssh ssh, struct Packet *pktin)
     int msglen;
 
     ssh_pkt_getstring(pktin, &msg, &msglen);
-    logeventf(ssh, "Remote debug message: %.*s", msglen, msg);
+    logeventf(ssh, "Remote debug message: %.*s", msglen, NULLTOEMPTY(msg));
 }
 
 static void ssh1_msg_disconnect(Ssh ssh, struct Packet *pktin)
@@ -5912,7 +5912,8 @@ static void ssh1_msg_disconnect(Ssh ssh, struct Packet *pktin)
     int msglen;
 
     ssh_pkt_getstring(pktin, &msg, &msglen);
-    bombout(("Server sent disconnect message:\n\"%.*s\"", msglen, msg));
+    bombout(("Server sent disconnect message:\n\"%.*s\"",
+             msglen, NULLTOEMPTY(msg)));
 }
 
 static void ssh_msg_ignore(Ssh ssh, struct Packet *pktin)
@@ -7960,7 +7961,8 @@ static void ssh2_msg_channel_open_failure(Ssh ssh, struct Packet *pktin)
             reason_code = 0; /* ensure reasons[reason_code] in range */
         ssh_pkt_getstring(pktin, &reason_string, &reason_length);
         logeventf(ssh, "Forwarded connection refused by server: %s [%.*s]",
-                  reasons[reason_code], reason_length, reason_string);
+                  reasons[reason_code], reason_length,
+                  NULLTOEMPTY(reason_string));
 
         pfd_close(c->u.pfd.pf);
     } else if (c->type == CHAN_ZOMBIE) {
@@ -8255,9 +8257,7 @@ static void ssh2_msg_channel_open(Ssh ssh, struct Packet *pktin)
        char *addrstr;
 
        ssh_pkt_getstring(pktin, &peeraddr, &peeraddrlen);
-       addrstr = snewn(peeraddrlen+1, char);
-       memcpy(addrstr, peeraddr, peeraddrlen);
-       addrstr[peeraddrlen] = '\0';
+       addrstr = dupprintf("%.*s", peeraddrlen, NULLTOEMPTY(peeraddr));
        peerport = ssh_pkt_getuint32(pktin);
 
        logeventf(ssh, "Received X11 connect request from %s:%d",
@@ -8292,13 +8292,14 @@ static void ssh2_msg_channel_open(Ssh ssh, struct Packet *pktin)
        char *shost;
        int shostlen;
        ssh_pkt_getstring(pktin, &shost, &shostlen);/* skip address */
-        pf.shost = dupprintf("%.*s", shostlen, shost);
+        pf.shost = dupprintf("%.*s", shostlen, NULLTOEMPTY(shost));
        pf.sport = ssh_pkt_getuint32(pktin);
        ssh_pkt_getstring(pktin, &peeraddr, &peeraddrlen);
        peerport = ssh_pkt_getuint32(pktin);
        realpf = find234(ssh->rportfwds, &pf, NULL);
        logeventf(ssh, "Received remote port %s:%d open request "
-                 "from %s:%d", pf.shost, pf.sport, peeraddr, peerport);
+                 "from %.*s:%d", pf.shost, pf.sport,
+                  peeraddrlen, NULLTOEMPTY(peeraddr), peerport);
         sfree(pf.shost);
 
        if (realpf == NULL) {
@@ -9957,7 +9958,7 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen,
                    s->cur_prompt->to_server = TRUE;
                    s->cur_prompt->name = dupstr("New SSH password");
                    s->cur_prompt->instruction =
-                       dupprintf("%.*s", prompt_len, prompt);
+                       dupprintf("%.*s", prompt_len, NULLTOEMPTY(prompt));
                    s->cur_prompt->instr_reqd = TRUE;
                    /*
                     * There's no explicit requirement in the protocol
@@ -10394,13 +10395,13 @@ static void ssh2_msg_disconnect(Ssh ssh, struct Packet *pktin)
     logevent(buf);
     sfree(buf);
     buf = dupprintf("Disconnection message text: %.*s",
-                   msglen, msg);
+                   msglen, NULLTOEMPTY(msg));
     logevent(buf);
     bombout(("Server sent disconnect message\ntype %d (%s):\n\"%.*s\"",
             reason,
             (reason > 0 && reason < lenof(ssh2_disconnect_reasons)) ?
             ssh2_disconnect_reasons[reason] : "unknown",
-            msglen, msg));
+            msglen, NULLTOEMPTY(msg)));
     sfree(buf);
 }
 
@@ -10414,7 +10415,7 @@ static void ssh2_msg_debug(Ssh ssh, struct Packet *pktin)
     ssh2_pkt_getbool(pktin);
     ssh_pkt_getstring(pktin, &msg, &msglen);
 
-    logeventf(ssh, "Remote debug message: %.*s", msglen, msg);
+    logeventf(ssh, "Remote debug message: %.*s", msglen, NULLTOEMPTY(msg));
 }
 
 static void ssh2_msg_transport(Ssh ssh, struct Packet *pktin)