diff --git a/misc.c b/misc.c index 1a528923..42a4a228 100644 --- a/misc.c +++ b/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 diff --git a/misc.h b/misc.h index 4e0d8b2d..b63df049 100644 --- a/misc.h +++ b/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); diff --git a/pscp.c b/pscp.c index 594f119f..67029f94 100644 --- a/pscp.c +++ b/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) { /* diff --git a/psftp.c b/psftp.c index d57b118e..4eb4068d 100644 --- a/psftp.c +++ b/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) { /* diff --git a/putty.h b/putty.h index dd43e8c4..5bb7974f 100644 --- a/putty.h +++ b/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); diff --git a/ssh.c b/ssh.c index 9ac60d27..c8365cae 100644 --- a/ssh.c +++ b/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); diff --git a/terminal.c b/terminal.c index a8bd6fc7..621d81de 100644 --- a/terminal.c +++ b/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; } -/* - * 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) +static void term_data_untrusted(Terminal *term, const void *data, int len) { - const char *data = (const char *)vdata; - int i; - /* 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 */ + sanitise_term_data(&term->inbuf, data, len); + term_added_data(term); } void term_provide_logctx(Terminal *term, LogContext *logctx) diff --git a/unix/gtkwin.c b/unix/gtkwin.c index 46b62782..eb3f201f 100644 --- a/unix/gtkwin.c +++ b/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 */ diff --git a/unix/uxcons.c b/unix/uxcons.c index b4fecce1..cf3603a4 100644 --- a/unix/uxcons.c +++ b/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++) - if ((data[i] & 0x60) || (data[i] == '\n')) - fputc(data[i], outfp); + bufchain_init(&sanitised); + sanitise_term_data(&sanitised, data, len); + while (bufchain_size(&sanitised) > 0) { + bufchain_prefix(&sanitised, &vdata, &len); + fwrite(vdata, 1, len, outfp); + bufchain_consume(&sanitised, len); + } fflush(outfp); } diff --git a/unix/uxplink.c b/unix/uxplink.c index 56bdf533..b058dcd0 100644 --- a/unix/uxplink.c +++ b/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); diff --git a/windows/wincons.c b/windows/wincons.c index 10367be8..34ed6a30 100644 --- a/windows/wincons.c +++ b/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 */ - WriteFile(hout, data, len, &dummy, NULL); + bufchain sanitised; + 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) diff --git a/windows/window.c b/windows/window.c index eb0f5e01..b73de782 100644 --- a/windows/window.c +++ b/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 */ diff --git a/windows/winplink.c b/windows/winplink.c index c156d740..c647a65a 100644 --- a/windows/winplink.c +++ b/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);