[Pkg-gnutls-maint] [KSBA] Suggested patch for fixing http://bugs.debian.org/391278 aka CVE-2006-5111 in sarge

Andreas Metzler ametzler at downhill.at.eu.org
Sat Oct 7 12:56:45 UTC 2006


Hej,

I've tried extracting the respective changes for fixing #391278 in
sarge from upstream's svn. Comments, feedback and testing would be very
welcome.

thanks, cu andreas
-- 
The 'Galactic Cleaning' policy undertaken by Emperor Zhark is a personal
vision of the emperor's, and its inclusion in this work does not constitute
tacit approval by the author or the publisher for any such projects,
howsoever undertaken.                                (c) Jasper Ffforde
-------------- next part --------------
diff -u libksba-0.9.9/debian/changelog libksba-0.9.9/debian/changelog
--- libksba-0.9.9/debian/changelog
+++ libksba-0.9.9/debian/changelog
@@ -1,3 +1,10 @@
+libksba (0.9.9-2sarge1) stable-security; urgency=high
+
+  * Fix crash via on malformed X.509 certificate in a signature.
+    CVE-2006-5111. Patch pulled from 0.9.12 --> 0.9.15. (Closes: #391278)
+
+ -- Andreas Metzler <ametzler at debian.org>  Sat,  7 Oct 2006 14:46:58 +0200
+
 libksba (0.9.9-2) unstable; urgency=medium
 
   * libksba-dev needs a Replaces: on libksba0 because the latter
only in patch2:
unchanged:
--- libksba-0.9.9.orig/src/ber-decoder.c
+++ libksba-0.9.9/src/ber-decoder.c
@@ -61,6 +61,19 @@
   AsnNode root;   /* of the expanded parse tree */
   DECODER_STATE ds;
   int bypass;
+
+  /* Because some certificates actually come with trailing garbage, we
+     use a hack to ignore this garbage.  This hack is enabled for data
+     starting with a fixed length sequence and this variable takes the
+     length of this sequence.  If it is 0, the hack is not
+     acticated. */
+  unsigned long outer_sequence_length;
+  int ignore_garbage;  /* Set to indicate that garpage should be
+                          ignored. */
+
+  int first_tag_seen;  /* Indicates whether the first tag of a decoder
+                          run has been read. */
+
   int honor_module_end; 
   int debug;
   int use_image;
@@ -733,7 +746,7 @@
 decoder_next (BerDecoder d)
 {
   struct tag_info ti;
-  AsnNode node;
+  AsnNode node = NULL;
   gpg_error_t err;
   DECODER_STATE ds = d->ds;
   int debug = d->debug;
@@ -741,6 +754,17 @@
   err = _ksba_ber_read_tl (d->reader, &ti);
   if (err)
     {
+      /* This is our actual hack to cope with some trailing garbage:
+         Only if we get an premature EOF and we know that we have read
+         the complete certificate we change the error to EOF.  This
+         won't help with all kinds of garbage but it fixes the case
+         where just one byte is appended.  This is for example the
+         case with current Siemens certificates.  This approach seems
+         to be the least intrusive one. */
+      if (gpg_err_code (err) == GPG_ERR_BAD_BER
+          && d->ignore_garbage
+          && ti.err_string && !strcmp (ti.err_string, "premature EOF"))
+        err = gpg_error (GPG_ERR_EOF);
       return err;
     }
 
@@ -749,6 +773,15 @@
       printf ("ReadTLV <"); dump_tlv (&ti, stdout); printf (">\n");
     }
 
+  /* Check whether the trailing garbage hack is required. */
+  if (!d->first_tag_seen)
+    {
+      d->first_tag_seen = 1;
+      if (ti.tag == TYPE_SEQUENCE && ti.length && !ti.ndef)
+        d->outer_sequence_length = ti.length;
+    }
+
+  /* Store stuff in the image buffer. */
   if (d->use_image)
     {
       if (!d->image.buf)
@@ -835,7 +868,15 @@
                             ds->idx? ds->stack[ds->idx-1].length:-1,
                             ds->cur.nread,
                             ti.is_constructed? "con":"pri");
-
+                  if (d->outer_sequence_length
+                      && ds->idx == 1
+                      && ds->cur.nread == d->outer_sequence_length)
+                    {
+                      if (debug)
+                        fprintf (stderr, "  Need to stop now\n");
+                      d->ignore_garbage = 1;
+                    }
+		  
                   if ( ds->idx
                        && !ds->stack[ds->idx-1].ndef_length
                        && (ds->cur.nread
@@ -864,7 +905,7 @@
                       && (ds->cur.nread
                           >= ds->stack[ds->idx-1].length));
                   
-              if (ti.is_constructed)
+              if (ti.is_constructed && (ti.length || ti.ndef))
                 {
                   /* prepare for the next level */
                   ds->cur.length = ti.length;
@@ -1070,9 +1111,12 @@
       int n, c;
 
       node = d->val.node;
-      if (node && d->use_image)
+      /* Fixme: USE_IMAGE is only not used with the ber-dump utility
+         and thus of no big use.  We should remove the other code
+         paths and dump ber-dump.c. */
+      if (d->use_image)
         {
-          if (!d->val.is_endtag)
+          if (node && !d->val.is_endtag)
             { /* We don't have nodes for the end tag - so don't store it */
               node->off = (ksba_reader_tell (d->reader)
                            - d->val.nhdr - startoff);


More information about the Pkg-gnutls-maint mailing list