From 243d12fdec66a4360fbb3e307a046b39b5b4ffc3 Mon Sep 17 00:00:00 2001
From: Werner Koch <wk@gnupg.org>
Date: Wed, 8 Apr 2015 18:51:21 +0200
Subject: [PATCH 1/3] Fix encoding of invalid utf-8 strings in dn.c
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/dn.c (append_quoted, append_atv): Use snprintf.
(append_utf8_value): Fix invalid encoding handling.
--

An invalid utf-8 encoding will make the loop in append_utf8_value run
once more with N > length which is not found by the termination
condition and only the former assert terminates the process if the byte
following the bad encoding has the high bit cleared.  This will lead
to a read access out of bounds.

The patch removes the assert and fixes the handling of bad encoding.
Due to the new quoting the output of a badly encoded utf-8 string will
be different than in previous versions.

Replacing sprintf is only for cosmetic reasons.

Use "gpgsm --verify FILE" to exhibit the problem.  FILE is
-----BEGIN PGP ARMORED FILE-----

MDAGCSqGSIb3DQEHAqCAMDACAQExDzANBgkwMDAwMDAwMDAwADCABgkwMDAwMDAw
MDAAMDEwAgEwMDAwMDEwMDAGA1UEAwwB/4AwMDAwMDAwMDAwMDAwMDAwMDAwMDAw
MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw
=NJTr
-----END PGP ARMORED FILE-----

Reported-by: Hanno Böck
Signed-off-by: Werner Koch <wk@gnupg.org>
---
 src/dn.c | 44 +++++++++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 17 deletions(-)

Index: libksba-1.0.4/src/dn.c
===================================================================
--- libksba-1.0.4.orig/src/dn.c
+++ libksba-1.0.4/src/dn.c
@@ -255,7 +255,7 @@ append_quoted (struct stringbuf *sb, con
       n += skip;
       if ( *s < ' ' || *s > 126 )
         {
-          sprintf (tmp, "\\%02X", *s);
+          snprintf (tmp, sizeof tmp, "\\%02X", *s);
           put_stringbuf_mem (sb, tmp, 3);
         }
       else
@@ -295,7 +295,6 @@ append_utf8_value (const unsigned char *
       length--;
     }
 
-  /* FIXME: check that the invalid encoding handling is correct */
   for (s=value, n=0;;)
     {
       for (value = s; n < length && !(*s & 0x80); n++, s++)
@@ -304,8 +303,9 @@ append_utf8_value (const unsigned char *
         append_quoted (sb, value, s-value, 0);
       if (n==length)
         return; /* ready */
-      assert ((*s & 0x80));
-      if ( (*s & 0xe0) == 0xc0 )      /* 110x xxxx */
+      if (!(*s & 0x80))
+        nmore = 0; /* Not expected here: high bit not set.  */
+      else if ( (*s & 0xe0) == 0xc0 ) /* 110x xxxx */
         nmore = 1;
       else if ( (*s & 0xf0) == 0xe0 ) /* 1110 xxxx */
         nmore = 2;
@@ -315,21 +315,28 @@ append_utf8_value (const unsigned char *
         nmore = 4;
       else if ( (*s & 0xfe) == 0xfc ) /* 1111 110x */
         nmore = 5;
-      else /* invalid encoding */
-        nmore = 5;  /* we will reduce the check length anyway */
-
-      if (n+nmore > length)
-        nmore = length - n; /* oops, encoding to short */ 
+      else /* Invalid encoding */
+        nmore = 0;
 
-      tmp[0] = *s++; n++;
-      for (i=1; i <= nmore; i++)
+      if (!nmore)
         {
-          if ( (*s & 0xc0) != 0x80)
-            break; /* invalid encoding - stop */
-          tmp[i] = *s++;
-          n++;
+          /* Encoding error:  We quote the bad byte.  */
+          snprintf (tmp, sizeof tmp, "\\%02X", *s);
+          put_stringbuf_mem (sb, tmp, 3);
+          s++; n++; 
+        }
+      else
+        {
+          tmp[0] = *s++; n++;
+          for (i=1; n < length && i <= nmore; i++)
+            {
+              if ( (*s & 0xc0) != 0x80)
+                break; /* Invalid encoding - let the next cycle detect this. */
+              tmp[i] = *s++;
+              n++;
+            }
+          put_stringbuf_mem (sb, tmp, i);
         }
-      put_stringbuf_mem (sb, tmp, i);
     }
 }
 
@@ -594,7 +601,7 @@ append_atv (const unsigned char *image,
       for (i=0; i < node->len; i++)
         { 
           char tmp[3];
-          sprintf (tmp, "%02X", image[node->off+node->nhdr+i]);
+          snprintf (tmp, sizeof tmp, "%02X", image[node->off+node->nhdr+i]);
           put_stringbuf (sb, tmp);
         }
       break;
