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

diff --git a/config.c b/config.c
index 84262f8..7e0a2fe 100644 (file)
--- 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 b08d6dd..0b5dcbb 100644 (file)
--- 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 fd1318d..b003652 100644 (file)
--- 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 5040fa2..0d3798a 100644 (file)
--- 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 f5603a8..0a03f3a 100644 (file)
--- 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);