From e65096f25904d6ac34414fb99d4ad62eb3fd823b Mon Sep 17 00:00:00 2001 From: jacob Date: Sun, 31 Dec 2006 15:33:33 +0000 Subject: [PATCH] Patch inspired by one from Daniel Silverstone in Debian bug #229232: We now have an option where a remote window title query returns a well-formed response containing the empty string. This should keep stop any server-side application that was expecting a response from hanging, while not permitting the response to be influenced by an attacker. We also retain the ability to stay schtum. The existing checkbox has thus grown into a set of radio buttons. I've changed the default to the "empty string" response, even in the backward- compatibility mode of loading old settings, which is a change in behaviour; any users who want the old behaviour back will have to explicitly select it. I think this is probably the Right Thing. (The only drawback I can think of is that an attacker could still potentially use the relevant fixed strings for mischief, but we already have other, similar reports.) git-svn-id: svn://svn.tartarus.org/sgt/putty@7043 cda61777-01e9-0310-a592-d414129be87e --- config.c | 10 +++++++--- doc/config.but | 26 +++++++++++++++++++++++--- putty.h | 7 ++++++- settings.c | 14 ++++++++++++-- terminal.c | 16 ++++++++++++---- 5 files changed, 60 insertions(+), 13 deletions(-) diff --git a/config.c b/config.c index 84262f8a..7e0a2fe6 100644 --- a/config.c +++ b/config.c @@ -1363,9 +1363,13 @@ void setup_config_box(struct controlbox *b, int midsession, HELPCTX(features_retitle), dlg_stdcheckbox_handler, I(offsetof(Config,no_remote_wintitle))); - ctrl_checkbox(s, "Disable remote window title querying (SECURITY)", - 'q', HELPCTX(features_qtitle), dlg_stdcheckbox_handler, - I(offsetof(Config,no_remote_qtitle))); + ctrl_radiobuttons(s, "Response to remote title query (SECURITY):", 'q', 3, + HELPCTX(features_qtitle), + dlg_stdradiobutton_handler, + I(offsetof(Config,remote_qtitle_action)), + "None", I(TITLE_NONE), + "Empty string", I(TITLE_EMPTY), + "Window title", I(TITLE_REAL), NULL); ctrl_checkbox(s, "Disable destructive backspace on server sending ^?",'b', HELPCTX(features_dbackspace), dlg_stdcheckbox_handler, I(offsetof(Config,no_dbackspace))); diff --git a/doc/config.but b/doc/config.but index b08d6dde..0b5dcbb9 100644 --- a/doc/config.but +++ b/doc/config.but @@ -882,7 +882,7 @@ commands from the server. If you find PuTTY is doing this unexpectedly or inconveniently, you can tell PuTTY not to respond to those server commands. -\S{config-features-qtitle} Disabling remote \i{window title} querying +\S{config-features-qtitle} Response to remote \i{window title} querying \cfg{winhelp-topic}{features.qtitle} @@ -899,8 +899,28 @@ service to have the new window title sent back to the server as if typed at the keyboard. This allows an attacker to fake keypresses and potentially cause your server-side applications to do things you didn't want. Therefore this feature is disabled by default, and we -recommend you do not turn it on unless you \e{really} know what you -are doing. +recommend you do not set it to \q{Window title} unless you \e{really} +know what you are doing. + +There are three settings for this option: + +\dt \q{None} + +\dd PuTTY makes no response whatsoever to the relevant escape +sequence. This may upset server-side software that is expecting some +sort of response. + +\dt \q{Empty string} + +\dd PuTTY makes a well-formed response, but leaves it blank. Thus, +server-side software that expects a response is kept happy, but an +attacker cannot influence the response string. This is probably the +setting you want if you have no better ideas. + +\dt \q{Window title} + +\dd PuTTY responds with the actual window title. This is dangerous for +the reasons described above. \S{config-features-dbackspace} Disabling \i{destructive backspace} diff --git a/putty.h b/putty.h index fd1318d0..b003652c 100644 --- a/putty.h +++ b/putty.h @@ -298,6 +298,11 @@ enum { }; enum { + /* Actions on remote window title query */ + TITLE_NONE, TITLE_EMPTY, TITLE_REAL +}; + +enum { /* Protocol back ends. (cfg.protocol) */ PROT_RAW, PROT_TELNET, PROT_RLOGIN, PROT_SSH, /* PROT_SERIAL is supported on a subset of platforms, but it doesn't @@ -486,7 +491,7 @@ struct config_tag { int no_remote_wintitle; /* disable remote retitling */ int no_dbackspace; /* disable destructive backspace */ int no_remote_charset; /* disable remote charset config */ - int no_remote_qtitle; /* disable remote win title query */ + int remote_qtitle_action; /* remote win title query action */ int app_cursor; int app_keypad; int nethack_keypad; diff --git a/settings.c b/settings.c index 5040fa25..0d3798aa 100644 --- a/settings.c +++ b/settings.c @@ -324,7 +324,7 @@ void save_open_settings(void *sesskey, int do_host, Config *cfg) write_setting_i(sesskey, "NoRemoteResize", cfg->no_remote_resize); write_setting_i(sesskey, "NoAltScreen", cfg->no_alt_screen); write_setting_i(sesskey, "NoRemoteWinTitle", cfg->no_remote_wintitle); - write_setting_i(sesskey, "NoRemoteQTitle", cfg->no_remote_qtitle); + write_setting_i(sesskey, "RemoteQTitleAction", cfg->remote_qtitle_action); write_setting_i(sesskey, "NoDBackspace", cfg->no_dbackspace); write_setting_i(sesskey, "NoRemoteCharset", cfg->no_remote_charset); write_setting_i(sesskey, "ApplicationCursorKeys", cfg->app_cursor); @@ -606,7 +606,17 @@ void load_open_settings(void *sesskey, int do_host, Config *cfg) gppi(sesskey, "NoRemoteResize", 0, &cfg->no_remote_resize); gppi(sesskey, "NoAltScreen", 0, &cfg->no_alt_screen); gppi(sesskey, "NoRemoteWinTitle", 0, &cfg->no_remote_wintitle); - gppi(sesskey, "NoRemoteQTitle", 1, &cfg->no_remote_qtitle); + { + /* Backward compatibility */ + int no_remote_qtitle; + gppi(sesskey, "NoRemoteQTitle", 1, &no_remote_qtitle); + /* We deliberately interpret the old setting of "no response" as + * "empty string". This changes the behaviour, but hopefully for + * the better; the user can always recover the old behaviour. */ + gppi(sesskey, "RemoteQTitleAction", + no_remote_qtitle ? TITLE_EMPTY : TITLE_REAL, + &cfg->remote_qtitle_action); + } gppi(sesskey, "NoDBackspace", 0, &cfg->no_dbackspace); gppi(sesskey, "NoRemoteCharset", 0, &cfg->no_remote_charset); gppi(sesskey, "ApplicationCursorKeys", 0, &cfg->app_cursor); diff --git a/terminal.c b/terminal.c index f5603a83..0a03f3a2 100644 --- a/terminal.c +++ b/terminal.c @@ -65,6 +65,8 @@ #define has_compat(x) ( ((CL_##x)&term->compatibility_level) != 0 ) +char *EMPTY_WINDOW_TITLE = ""; + const char sco2ansicolour[] = { 0, 4, 2, 6, 1, 5, 3, 7 }; #define sel_nl_sz (sizeof(sel_nl)/sizeof(wchar_t)) @@ -3791,8 +3793,11 @@ static void term_out(Terminal *term) break; case 20: if (term->ldisc && - !term->cfg.no_remote_qtitle) { - p = get_window_title(term->frontend, TRUE); + term->cfg.remote_qtitle_action != TITLE_NONE) { + if(term->cfg.remote_qtitle_action == TITLE_REAL) + p = get_window_title(term->frontend, TRUE); + else + p = EMPTY_WINDOW_TITLE; len = strlen(p); ldisc_send(term->ldisc, "\033]L", 3, 0); ldisc_send(term->ldisc, p, len, 0); @@ -3801,8 +3806,11 @@ static void term_out(Terminal *term) break; case 21: if (term->ldisc && - !term->cfg.no_remote_qtitle) { - p = get_window_title(term->frontend,FALSE); + term->cfg.remote_qtitle_action != TITLE_NONE) { + if(term->cfg.remote_qtitle_action == TITLE_REAL) + p = get_window_title(term->frontend, FALSE); + else + p = EMPTY_WINDOW_TITLE; len = strlen(p); ldisc_send(term->ldisc, "\033]l", 3, 0); ldisc_send(term->ldisc, p, len, 0); -- 2.11.0