OpenSSH banner doesnot display multibyte characters like korean

[balu chandra]

I did not come across any UTF8 implementation of strnvis.
Howver have modified strnvis() to display Multibyte char (tested the
fix with Korean char).
Though all calls to strnvis() could be extended to skip Multibyte char
this fix is limited to skipping only for Banner message(introduced a
flag for doing so).

The fix is provided here for comments.

[root at rhel62 bala]# diff -ur openssh6.1 openssh6.1_mod
diff -ur openssh6.1/openbsd-compat/vis.c openssh6.1_mod/openbsd-compat/vis.c
--- openssh6.1/openbsd-compat/vis.c     2012-11-27 16:12:23.986690241 +0530
+++ openssh6.1_mod/openbsd-compat/vis.c 2012-11-27 16:12:43.559859183 +0530
@@ -50,6 +50,19 @@
                (c) == '\007' || (c) == '\r' ||                         \
                isgraph((u_char)(c)))))

+int multi_byte_banner = 0;
+#ifdef WORDS_BIGENDIAN
+#define FIRST_BYTE 0
+#define SECOND_BYTE 1
+#define THIRD_BYTE 2
+#define FOURTH_BYTE 3
+#else
+#define FIRST_BYTE 3
+#define SECOND_BYTE 2
+#define THIRD_BYTE 1
+#define FOURTH_BYTE 0
+#endif
+
 /*
  * vis - visually encode characters
  */
@@ -168,9 +181,15 @@
        char *start, *end;
        char tbuf[5];
        int c, i;
+       int multi_byte_num;

        i = 0;
        for (start = dst, end = start + siz - 1; (c = *src) && dst < end; ) {
+               multi_byte_num = 0;
+/*         if ( ( 0300 & c ) == 0200 ) {
+               *dst++ = c;
+               src++;
+           }     */
                if (isvisible(c)) {
                        i = 1;
                        *dst++ = c;
@@ -185,6 +204,33 @@
                                }
                        }
                        src++;
+/* Though ISO defines 6 bytes for UTF8, a valid unicode char is
defined by 1-4 byte UTF8 sequence
+  For a one byte UTF8 char:
+                          same as ASCII (0x00 - 0x7F)
+       two byte UTF8 char:
+                          1st byte is bit-110?????
+                          2nd byte is bit-10??????
+       three byte UTF8 char:
+                          1st byte is bit-1110????
+                          2nd and 3rd bytes are bit-10??????
+        four byte UTF8 char:
+                           1st byte is bit-11110???
+                           2nd, 3rd and 4th bytes are bit-10??????  */
+               } else if ( multi_byte_banner && (((*src+FIRST_BYTE) &
0xE0) == 0xC0) &&
+                               ((*(src+SECOND_BYTE) & 0xC0) == 0x80) ) {
+                       /* 2-byte char   */
+                       multi_byte_num = 2;
+                } else if ( multi_byte_banner && (((*src+FIRST_BYTE)
& 0xF0) == 0xE0) &&
+                               ((*(src+SECOND_BYTE) & 0xC0) == 0x80) &&
+                               ((*(src+THIRD_BYTE) & 0xC0) == 0x80) ) {
+                       /* 3-byte char   */
+                       multi_byte_num = 3;
+               } else if ( multi_byte_banner && (((*src+FIRST_BYTE) &
0xF1) == 0xF0) &&
+                               ((*(src+SECOND_BYTE) & 0xC0) == 0x80) &&
+                               ((*(src+THIRD_BYTE) & 0xC0) == 0x80) &&
+                               ((*(src+FOURTH_BYTE) & 0xC0) == 0x80) ) {
+                       /* 4-byte char   */
+                       multi_byte_num = 4;
                } else {
                        i = vis(tbuf, c, flag, *++src) - tbuf;
                        if (dst + i <= end) {
@@ -195,6 +241,9 @@
                                break;
                        }
                }
+               while (multi_byte_num--) {
+                       *dst++ = *src++;
+               }
        }
        if (siz > 0)
                *dst = '\0';
@@ -203,6 +252,7 @@
                while ((c = *src))
                        dst += vis(tbuf, c, flag, *++src) - tbuf;
        }
+       multi_byte_banner = 0;
        return (dst - start);
 }

diff -ur openssh6.1/sshconnect2.c openssh6.1_mod/sshconnect2.c
--- openssh6.1/sshconnect2.c    2012-11-27 16:11:34.084649708 +0530
+++ openssh6.1_mod/sshconnect2.c        2012-11-27 16:10:58.950588684 +0530
@@ -103,6 +103,7 @@

 Kex *xxx_kex = NULL;

+extern int multi_byte_banner;
 static int
 verify_host_key_callback(Key *hostkey)
 {
@@ -535,6 +536,7 @@
                if (len > 65536)
                        len = 65536;
                msg = xmalloc(len * 4 + 1); /* max expansion from strnvis() */
+               multi_byte_banner = 1;
                strnvis(msg, raw, len * 4 + 1, VIS_SAFE|VIS_OCTAL|VIS_NOSLASH);
                fprintf(stderr, "%s", msg);
                xfree(msg);



Regards,
Bala

On 10/5/12, Darren Tucker <dtucker at zip.com.au> wrote:
> On Tue, Sep 25, 2012 at 9:12 PM, balu chandra <balu9463 at gmail.com> wrote:
>> I also found little information inthe changelog on why strnvis() was
>> introduced in input_userauth_banner. Is it added to address any
>> security vulnerability.
>
> I believe the intent was to prevent a malicious server from sending a
> banner containing a terminal answerback command sequence.  I'm not
> aware of any UTF-8 aware equivalent of strnvis, though (if someone
> knows of one we'll look at using it).
>
> --
> Darren Tucker (dtucker at zip.com.au)
> GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
>     Good judgement comes with experience. Unfortunately, the experience
> usually comes from bad judgement.
>