Rework handling of untrusted terminal data.

Now there's a centralised routine in misc.c to do the sanitisation,
which copies data on to an outgoing bufchain. This allows me to remove
from_backend_untrusted() completely from the frontend API, simplifying
code in several places.

Two use cases for untrusted-terminal-data sanitisation were in the
terminal.c prompts handler, and in the collection of SSH-2 userauth
banners. Both of those were writing output to a bufchain anyway, so
it was very convenient to just replace a bufchain_add with
sanitise_term_data and then not have to worry about it again.

There was also a simplistic sanitiser in uxcons.c, which I've now
replaced with a call to the good one - and in wincons.c there was a
FIXME saying I ought to get round to that, which now I have!

misc.c

@@ -826,6 +826,33 @@ int bufchain_try_fetch_consume(bufchain *ch, void *data, int len)
     }
 }
 
+/* ----------------------------------------------------------------------
+ * Sanitise terminal output that we have reason not to trust, e.g.
+ * because it appears in the login banner or password prompt from a
+ * server, which we'd rather not permit to use arbitrary escape
+ * sequences.
+ */
+
+void sanitise_term_data(bufchain *out, const void *vdata, int len)
+{
+    const char *data = (const char *)vdata;
+    int i;
+
+    /*
+     * FIXME: this method of sanitisation is ASCII-centric. It would
+     * be nice to permit SSH banners and the like to contain printable
+     * Unicode, but that would need a lot more complicated code here
+     * (not to mention knowing what character set it should interpret
+     * the data as).
+     */
+    for (i = 0; i < len; i++) {
+        if (data[i] == '\n')
+            bufchain_add(out, "\r\n", 2);
+        else if (data[i] >= ' ' && data[i] < 0x7F)
+            bufchain_add(out, data + i, 1);
+    }
+}
+
 /* ----------------------------------------------------------------------
  * My own versions of malloc, realloc and free. Because I want
  * malloc and realloc to bomb out and exit the program if they run

misc.h

@@ -90,6 +90,8 @@ void bufchain_fetch(bufchain *ch, void *data, int len);
 void bufchain_fetch_consume(bufchain *ch, void *data, int len);
 int bufchain_try_fetch_consume(bufchain *ch, void *data, int len);
 
+void sanitise_term_data(bufchain *out, const void *vdata, int len);
+
 int validate_manual_hostkey(char *key);
 
 struct tm ltime(void);

pscp.c

@@ -196,15 +196,6 @@ int from_backend(Frontend *frontend, int is_stderr,
 
     return 0;
 }
-int from_backend_untrusted(Frontend *frontend, const void *data, int len)
-{
-    /*
-     * No "untrusted" output should get here (the way the code is
-     * currently, it's all diverted by FLAG_STDERR).
-     */
-    assert(!"Unexpected call to from_backend_untrusted()");
-    return 0; /* not reached */
-}
 int from_backend_eof(Frontend *frontend)
 {
     /*

psftp.c

@@ -2551,15 +2551,6 @@ int from_backend(Frontend *frontend, int is_stderr,
 
     return 0;
 }
-int from_backend_untrusted(Frontend *frontend, const void *data, int len)
-{
-    /*
-     * No "untrusted" output should get here (the way the code is
-     * currently, it's all diverted by FLAG_STDERR).
-     */
-    assert(!"Unexpected call to from_backend_untrusted()");
-    return 0; /* not reached */
-}
 int from_backend_eof(Frontend *frontend)
 {
     /*

putty.h

@@ -712,7 +712,6 @@ void frontend_echoedit_update(Frontend *frontend, int echo, int edit);
  * shutdown. */
 void update_specials_menu(Frontend *frontend);
 int from_backend(Frontend *frontend, int is_stderr, const void *data, int len);
-int from_backend_untrusted(Frontend *frontend, const void *data, int len);
 /* Called when the back end wants to indicate that EOF has arrived on
  * the server-to-client stream. Returns FALSE to indicate that we
  * intend to keep the session open in the other direction, or TRUE to
@@ -1130,7 +1129,6 @@ void term_request_copy(Terminal *, const int *clipboards, int n_clipboards);
 void term_request_paste(Terminal *, int clipboard);
 void term_seen_key_event(Terminal *); 
 int term_data(Terminal *, int is_stderr, const void *data, int len);
-int term_data_untrusted(Terminal *, const void *data, int len);
 void term_provide_backend(Terminal *term, Backend *backend);
 void term_provide_logctx(Terminal *term, LogContext *logctx);
 void term_set_focus(Terminal *term, int has_focus);

ssh.c

@@ -1078,14 +1078,6 @@ static void c_write(Ssh ssh, const void *buf, int len)
 	from_backend(ssh->frontend, 1, buf, len);
 }
 
-static void c_write_untrusted(Ssh ssh, const void *buf, int len)
-{
-    if (flags & FLAG_STDERR)
-	c_write_stderr(0, buf, len);
-    else
-	from_backend_untrusted(ssh->frontend, buf, len);
-}
-
 static void c_write_str(Ssh ssh, const char *buf)
 {
     c_write(ssh, buf, strlen(buf));
@@ -7066,7 +7058,7 @@ static void ssh2_msg_userauth_banner(Ssh ssh, PktIn *pktin)
 	bufchain_size(&ssh->banner) <= 131072) {
 	ptrlen banner = get_string(pktin);
 	if (banner.len)
-	    bufchain_add(&ssh->banner, banner.ptr, banner.len);
+            sanitise_term_data(&ssh->banner, banner.ptr, banner.len);
     }
 }
 
@@ -7679,11 +7671,15 @@ static void do_ssh2_userauth(void *vctx)
 		 * banner _anyway_, and moreover the printing of
 		 * the banner will screw up processing on the
 		 * output of (say) plink.)
+                 *
+                 * The banner data has been sanitised already by this
+                 * point, so we can safely send it straight to the
+                 * output channel.
 		 */
 		if (size && (flags & (FLAG_VERBOSE | FLAG_INTERACTIVE))) {
 		    char *banner = snewn(size, char);
 		    bufchain_fetch(&ssh->banner, banner, size);
-		    c_write_untrusted(ssh, banner, size);
+                    c_write(ssh, banner, size);
 		    sfree(banner);
 		}
 		bufchain_clear(&ssh->banner);

terminal.c

@@ -6624,10 +6624,8 @@ int term_ldisc(Terminal *term, int option)
     return FALSE;
 }
 
-int term_data(Terminal *term, int is_stderr, const void *data, int len)
+static void term_added_data(Terminal *term)
 {
-    bufchain_add(&term->inbuf, data, len);
-
     if (!term->in_term_out) {
 	term->in_term_out = TRUE;
 	term_reset_cblink(term);
@@ -6640,6 +6638,12 @@ int term_data(Terminal *term, int is_stderr, const void *data, int len)
 	    term_out(term);
 	term->in_term_out = FALSE;
     }
+}
+
+int term_data(Terminal *term, int is_stderr, const void *data, int len)
+{
+    bufchain_add(&term->inbuf, data, len);
+    term_added_data(term);
 
     /*
      * term_out() always completely empties inbuf. Therefore,
@@ -6663,23 +6667,10 @@ int term_data(Terminal *term, int is_stderr, const void *data, int len)
     return 0;
 }
 
-/*
+static void term_data_untrusted(Terminal *term, const void *data, int len)
- * Write untrusted data to the terminal.
- * The only control character that should be honoured is \n (which
- * will behave as a CRLF).
- */
-int term_data_untrusted(Terminal *term, const void *vdata, int len)
 {
-    const char *data = (const char *)vdata;
+    sanitise_term_data(&term->inbuf, data, len);
-    int i;
+    term_added_data(term);
-    /* FIXME: more sophisticated checking? */
-    for (i = 0; i < len; i++) {
-	if (data[i] == '\n')
-	    term_data(term, 1, "\r\n", 2);
-	else if (data[i] & 0x60)
-	    term_data(term, 1, data + i, 1);
-    }
-    return 0; /* assumes that term_data() always returns 0 */
 }
 
 void term_provide_logctx(Terminal *term, LogContext *logctx)

unix/gtkwin.c

@@ -305,11 +305,6 @@ int from_backend(Frontend *inst, int is_stderr, const void *data, int len)
     return term_data(inst->term, is_stderr, data, len);
 }
 
-int from_backend_untrusted(Frontend *inst, const void *data, int len)
-{
-    return term_data_untrusted(inst->term, data, len);
-}
-
 int from_backend_eof(Frontend *inst)
 {
     return TRUE;   /* do respond to incoming EOF with outgoing */

unix/uxcons.c

@@ -448,11 +448,16 @@ static void console_close(FILE *outfp, int infd)
 
 static void console_prompt_text(FILE *outfp, const char *data, int len)
 {
-    int i;
+    bufchain sanitised;
+    void *vdata;
 
-    for (i = 0; i < len; i++)
+    bufchain_init(&sanitised);
-	if ((data[i] & 0x60) || (data[i] == '\n'))
+    sanitise_term_data(&sanitised, data, len);
-	    fputc(data[i], outfp);
+    while (bufchain_size(&sanitised) > 0) {
+        bufchain_prefix(&sanitised, &vdata, &len);
+        fwrite(vdata, 1, len, outfp);
+        bufchain_consume(&sanitised, len);
+    }
     fflush(outfp);
 }
 

unix/uxplink.c

@@ -413,16 +413,6 @@ int from_backend(Frontend *frontend, int is_stderr,
     }
 }
 
-int from_backend_untrusted(Frontend *frontend, const void *data, int len)
-{
-    /*
-     * No "untrusted" output should get here (the way the code is
-     * currently, it's all diverted by FLAG_STDERR).
-     */
-    assert(!"Unexpected call to from_backend_untrusted()");
-    return 0; /* not reached */
-}
-
 int from_backend_eof(Frontend *frontend)
 {
     assert(outgoingeof == EOF_NO);

windows/wincons.c

@@ -348,8 +348,16 @@ void logevent(Frontend *frontend, const char *string)
 static void console_data_untrusted(HANDLE hout, const char *data, int len)
 {
     DWORD dummy;
-    /* FIXME: control-character filtering */
+    bufchain sanitised;
-    WriteFile(hout, data, len, &dummy, NULL);
+    void *vdata;
+
+    bufchain_init(&sanitised);
+    sanitise_term_data(&sanitised, data, len);
+    while (bufchain_size(&sanitised) > 0) {
+        bufchain_prefix(&sanitised, &vdata, &len);
+        WriteFile(hout, vdata, len, &dummy, NULL);
+        bufchain_consume(&sanitised, len);
+    }
 }
 
 int console_get_userpass_input(prompts_t *p)

windows/window.c

@@ -5922,11 +5922,6 @@ int from_backend(Frontend *frontend, int is_stderr, const void *data, int len)
     return term_data(term, is_stderr, data, len);
 }
 
-int from_backend_untrusted(Frontend *frontend, const void *data, int len)
-{
-    return term_data_untrusted(term, data, len);
-}
-
 int from_backend_eof(Frontend *frontend)
 {
     return TRUE;   /* do respond to incoming EOF with outgoing */

windows/winplink.c

@@ -114,16 +114,6 @@ int from_backend(Frontend *frontend, int is_stderr,
     return handle_backlog(stdout_handle) + handle_backlog(stderr_handle);
 }
 
-int from_backend_untrusted(Frontend *frontend, const void *data, int len)
-{
-    /*
-     * No "untrusted" output should get here (the way the code is
-     * currently, it's all diverted by FLAG_STDERR).
-     */
-    assert(!"Unexpected call to from_backend_untrusted()");
-    return 0; /* not reached */
-}
-
 int from_backend_eof(Frontend *frontend)
 {
     handle_write_eof(stdout_handle);