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)));
                  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)));
     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.
 
 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}
 
 
 \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
 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}
 
 
 \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 {
 };
 
 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
     /* 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_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;
     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, "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);
     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, "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);
     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 )
 
 
 #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))
 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 &&
                                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);
                                    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 &&
                                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);
                                    len = strlen(p);
                                    ldisc_send(term->ldisc, "\033]l", 3, 0);
                                    ldisc_send(term->ldisc, p, len, 0);