Re: [rootshell.com] Xterm DoS Attack

[anakin () POBOX COM (Simon Tatham)]

wakko () WTOWER COM writes:
[xterm DoS through terminal-size sequences]
> This breaks PuTTY [a Win32 SSH client] as well.

Yesterday I checked in a fix to the PuTTY master CVS repository, and
last night's automated build ran successfully. This morning's PuTTY
development snapshot appears to be OK.

I wasn't able to reproduce the exploit using the same escape
sequence as in the xterm-destroying example code; on the other hand,
PuTTY was vulnerable to other sequences in the same spirit.

Philosophically, I have a hard time seeing this as a bug in any
given terminal emulator. There _should_ be a way for a (trusted) app
running in a terminal emulator to request window size changes and
other such things; it's very useful. And in the absence of separated
control and data streams within a terminal session (in which case
one could allow `cat' unrestricted access to the data stream and it
would not be able to DoS by injecting malice into the control
stream), the whole terminal session must be considered to be the
control stream, and vulnerable. Don't `cat' untrusted files.

(Of course, APC in MS-Kermit is possibly the worst exploit of this
type I've _ever_ seen. With a well-chosen escape sequence you could
direct the MS-Kermit program to run arbitrary commands on its host
machine...)

Anyway. Current development snapshots of PuTTY are now believed
robust. A 0.49 release containing the fix will be coming out RSN,
since security is the one thing that really makes me get off my butt
and put new releases out _quickly_ :-)

The patch is quoted below.

---------- begin patch ----------
Index: src/putty/terminal.c
===================================================================
RCS file: /home/cvs/putty/terminal.c,v
retrieving revision 1.18
retrieving revision 1.19
diff -u -r1.18 -r1.19
--- src/putty/terminal.c        2000/03/17 10:46:59     1.18
+++ src/putty/terminal.c        2000/06/05 16:33:58     1.19
@@ -1209,7 +1209,11 @@
                  */
                 compatibility(VT340TEXT);
                 if (esc_nargs<=1 && (esc_args[0]<1 || esc_args[0]>=24)) {
-                   request_resize (cols, def(esc_args[0], 24), 0);
+                   unsigned int newrows = def(esc_args[0], 24);
+                   /* Hack: prevent big-resize DoS attack. */
+                   if (newrows > max(512, cfg.height))
+                       newrows = max(512, cfg.height);
+                   request_resize (cols, newrows, 0);
                     deselect();
                 }
                 break;
@@ -1221,7 +1225,11 @@
                  */
                 compatibility(VT420);
                 if (esc_nargs==1 && esc_args[0]>=24) {
-                   request_resize (cols, def(esc_args[0], cfg.height), 0);
+                   unsigned int newrows = def(esc_args[0], cfg.height);
+                   /* Hack: prevent big-resize DoS attack. */
+                   if (newrows > max(512, cfg.height))
+                       newrows = max(512, cfg.height);
+                   request_resize (cols, newrows, 0);
                     deselect();
                 }
                 break;
@@ -1232,7 +1240,11 @@
                  */
                 compatibility(VT340TEXT);
                 if (esc_nargs<=1) {
-                   request_resize (cols, def(esc_args[0], cfg.width), 0);
+                   unsigned int newcols = def(esc_args[0], cfg.width);
+                   /* Hack: prevent big-resize DoS attack. */
+                   if (newcols > max(512, cfg.width))
+                       newcols = max(512, cfg.width);
+                   request_resize (newcols, rows, 0);
                     deselect();
                 }
                 break;
----------- end patch -----------

Cheers,
Simon

--
Simon Tatham         "infinite loop _see_ loop, infinite"
<anakin () pobox com>     - Index, Borland Pascal Language Guide