Bug#770972: libksba: buffer overflow in ksba_oid_to_str

Salvatore Bonaccorso carnil at debian.org
Tue Nov 25 18:11:25 UTC 2014


Hi

Attached both debdiffs for wheezy-security and unstable using the
upstream patch.

Regards,
Salvatore
-------------- next part --------------
diff -Nru libksba-1.2.0/debian/changelog libksba-1.2.0/debian/changelog
--- libksba-1.2.0/debian/changelog	2011-06-19 14:03:02.000000000 +0200
+++ libksba-1.2.0/debian/changelog	2014-11-25 16:49:50.000000000 +0100
@@ -1,3 +1,11 @@
+libksba (1.2.0-2+deb7u1) wheezy-security; urgency=high
+
+  * Non-maintainer upload by the Security Team.
+  * Add 0001-Fix-buffer-overflow-in-ksba_oid_to_str.patch patch.
+    Fix buffer overflow in ksba_oid_to_str. (Closes: #770972)
+
+ -- Salvatore Bonaccorso <carnil at debian.org>  Tue, 25 Nov 2014 16:43:46 +0100
+
 libksba (1.2.0-2) unstable; urgency=low
 
   * Build for multi-arch.
diff -Nru libksba-1.2.0/debian/patches/0001-Fix-buffer-overflow-in-ksba_oid_to_str.patch libksba-1.2.0/debian/patches/0001-Fix-buffer-overflow-in-ksba_oid_to_str.patch
--- libksba-1.2.0/debian/patches/0001-Fix-buffer-overflow-in-ksba_oid_to_str.patch	1970-01-01 01:00:00.000000000 +0100
+++ libksba-1.2.0/debian/patches/0001-Fix-buffer-overflow-in-ksba_oid_to_str.patch	2014-11-25 16:49:50.000000000 +0100
@@ -0,0 +1,209 @@
+Description: Fix buffer overflow in ksba_oid_to_str
+ The code has an obvious error by not considering invalid encoding for
+ arc-2.  A first byte of 0x80 can be used to make a value of less then
+ 80 and we then subtract 80 from that value as required by the OID
+ encoding rules.  Due to the unsigned integer this results in a pretty
+ long value which won't fit anymore into the allocated buffer.
+ .
+ The fix is obvious.  Also added a few simple test cases.  Note that we
+ keep on using sprintf instead of snprintf because managing the
+ remaining length of the buffer would probably be more error prone than
+ assuring that the buffer is large enough.  Getting rid of sprintf
+ altogether by using direct conversion along with membuf_t like code
+ might be possible.
+Origin: backport, http://git.gnupg.org/cgi-bin/gitweb.cgi?p=libksba.git;a=commit;h=f715b9e156dfa99ae829fc694e5a0abd23ef97d7
+Bug-Debian: https://bugs.debian.org/770972
+Forwarded: not-needed
+Author: Salvatore Bonaccorso <carnil at debian.org>
+Last-Update: 2014-11-25
+Applied-Upstream: 1.3.2
+
+--- a/tests/t-dnparser.c
++++ b/tests/t-dnparser.c
+@@ -144,7 +144,7 @@ main (int argc, char **argv)
+       if (!feof (stdin))
+         fail ("read error or input too large");
+ 
+-      fail ("no yet implemented");
++      fail ("not yet implemented");
+ 
+     }
+   else if (argc == 2 && !strcmp (argv[1], "--to-der") )
+--- a/tests/t-oid.c
++++ b/tests/t-oid.c
+@@ -27,6 +27,9 @@
+ 
+ #include "../src/ksba.h"
+ 
++#define PGM "t-oid"
++#define BADOID "1.3.6.1.4.1.11591.2.12242973"
++
+ 
+ static void *
+ read_into_buffer (FILE *fp, size_t *r_length)
+@@ -68,23 +71,104 @@ read_into_buffer (FILE *fp, size_t *r_le
+ }
+ 
+ 
++static void
++test_oid_to_str (void)
++{
++  struct {
++    unsigned int binlen;
++    unsigned char *bin;
++    char *str;
++  } tests[] = {
++
++    {  7, "\x02\x82\x06\x01\x0A\x0C\x00",
++       "0.2.262.1.10.12.0"
++    },
++    {  7, "\x02\x82\x06\x01\x0A\x0C\x01",
++       "0.2.262.1.10.12.1"
++    },
++    {  7, "\x2A\x86\x48\xCE\x38\x04\x01",
++       "1.2.840.10040.4.1"
++    },
++    {  7, "\x2A\x86\x48\xCE\x38\x04\x03",
++       "1.2.840.10040.4.3"
++    },
++    { 10, "\x2B\x06\x01\x04\x01\xDA\x47\x02\x01\x01",
++      "1.3.6.1.4.1.11591.2.1.1"
++    },
++    {  3, "\x55\x1D\x0E",
++       "2.5.29.14"
++    },
++    {  9, "\x80\x02\x70\x50\x25\x46\xfd\x0c\xc0",
++       BADOID
++    },
++    {  1, "\x80",
++       BADOID
++    },
++    {  2, "\x81\x00",
++       "2.48"
++    },
++    {  2, "\x81\x01",
++       "2.49"
++    },
++    {  2, "\x81\x7f",
++       "2.175"
++    },
++    {  2, "\x81\x80",  /* legal encoding? */
++       "2.48"
++    },
++    {  2, "\x81\x81\x01",  /* legal encoding? */
++       "2.49"
++    },
++    {  0, "",
++       ""
++    },
++
++    { 0, NULL, NULL }
++  };
++  int tidx;
++  char *str;
++
++  for (tidx=0; tests[tidx].bin; tidx++)
++    {
++      str = ksba_oid_to_str (tests[tidx].bin, tests[tidx].binlen);
++      if (!str)
++        {
++          perror ("ksba_oid_to_str failed");
++          exit (1);
++        }
++      if (strcmp (tests[tidx].str, str))
++        {
++          fprintf (stderr, "ksba_oid_to_str test %d failed\n", tidx);
++          fprintf (stderr, "  got=%s\n", str);
++          fprintf (stderr, " want=%s\n", tests[tidx].str);
++          exit (1);
++        }
++    }
++}
++
+ 
+ int
+ main (int argc, char **argv)
+ {
+   gpg_error_t err;
++
+   if (argc)
+     {
+       argc--;
+       argv++;
+     }
+ 
+-  if (argc)
++
++  if (!argc)
++    {
++      test_oid_to_str ();
++    }
++  else if (!strcmp (*argv, "--from-str"))
+     {
+       unsigned char *buffer;
+       size_t n, buflen;
+ 
+-      for ( ;argc ; argc--, argv++)
++      for (argv++,argc-- ; argc; argc--, argv++)
+         {
+           err = ksba_oid_from_str (*argv, &buffer, &buflen);
+           if (err)
+@@ -100,18 +184,25 @@ main (int argc, char **argv)
+           free (buffer);
+         }
+     }
+-  else
++  else if (!strcmp (*argv, "--to-str"))
+     {
+       char *buffer;
+       size_t buflen;
+       char *result;
+ 
++      argv++;argc--;
++
+       buffer = read_into_buffer (stdin, &buflen);
+       result = ksba_oid_to_str (buffer, buflen);
+       free (buffer);
+       printf ("%s\n", result? result:"[malloc failed]");
+       free (result);
+     }
++  else
++    {
++      fputs ("usage: "PGM" [--from-str|--to-str]\n", stderr);
++      return 1;
++    }
+ 
+   return 0;
+ }
+--- a/tests/Makefile.am
++++ b/tests/Makefile.am
+@@ -39,12 +39,12 @@ EXTRA_DIST = $(test_certs) samples/READM
+ BUILT_SOURCES = oidtranstbl.h
+ CLEANFILES = oidtranstbl.h
+ 
+-TESTS = cert-basic t-crl-parser t-dnparser
++TESTS = cert-basic t-crl-parser t-dnparser t-oid
+ 
+ AM_CFLAGS = $(GPG_ERROR_CFLAGS)
+ 
+ noinst_HEADERS = t-common.h
+-noinst_PROGRAMS = $(TESTS) t-cms-parser t-crl-parser t-dnparser t-ocsp t-oid
++noinst_PROGRAMS = $(TESTS) t-cms-parser t-crl-parser t-dnparser t-ocsp
+ LDADD = ../src/libksba.la $(GPG_ERROR_LIBS)
+ 
+ t_ocsp_SOURCES = t-ocsp.c sha1.c
+--- a/tests/Makefile.in
++++ b/tests/Makefile.in
+@@ -51,10 +51,9 @@ PRE_UNINSTALL = :
+ POST_UNINSTALL = :
+ build_triplet = @build@
+ host_triplet = @host@
+-TESTS = cert-basic$(EXEEXT) t-crl-parser$(EXEEXT) t-dnparser$(EXEEXT)
++TESTS = cert-basic$(EXEEXT) t-crl-parser$(EXEEXT) t-dnparser$(EXEEXT) t-oid$(EXEEXT)
+ noinst_PROGRAMS = $(am__EXEEXT_1) t-cms-parser$(EXEEXT) \
+-	t-crl-parser$(EXEEXT) t-dnparser$(EXEEXT) t-ocsp$(EXEEXT) \
+-	t-oid$(EXEEXT)
++	t-crl-parser$(EXEEXT) t-dnparser$(EXEEXT) t-ocsp$(EXEEXT)
+ subdir = tests
+ DIST_COMMON = $(noinst_HEADERS) $(srcdir)/Makefile.am \
+ 	$(srcdir)/Makefile.in ChangeLog
diff -Nru libksba-1.2.0/debian/patches/series libksba-1.2.0/debian/patches/series
--- libksba-1.2.0/debian/patches/series	1970-01-01 01:00:00.000000000 +0100
+++ libksba-1.2.0/debian/patches/series	2014-11-25 16:49:50.000000000 +0100
@@ -0,0 +1 @@
+0001-Fix-buffer-overflow-in-ksba_oid_to_str.patch
-------------- next part --------------
diff -Nru libksba-1.3.1/debian/changelog libksba-1.3.1/debian/changelog
--- libksba-1.3.1/debian/changelog	2014-09-26 19:27:40.000000000 +0200
+++ libksba-1.3.1/debian/changelog	2014-11-25 17:52:39.000000000 +0100
@@ -1,3 +1,11 @@
+libksba (1.3.1-1.1) unstable; urgency=medium
+
+  * Non-maintainer upload.
+  * Add 0001-Fix-buffer-overflow-in-ksba_oid_to_str.patch patch.
+    Fix buffer overflow in ksba_oid_to_str. (Closes: #770972)
+
+ -- Salvatore Bonaccorso <carnil at debian.org>  Tue, 25 Nov 2014 17:51:44 +0100
+
 libksba (1.3.1-1) unstable; urgency=medium
 
   * New upstream bugfix release.
diff -Nru libksba-1.3.1/debian/patches/0001-Fix-buffer-overflow-in-ksba_oid_to_str.patch libksba-1.3.1/debian/patches/0001-Fix-buffer-overflow-in-ksba_oid_to_str.patch
--- libksba-1.3.1/debian/patches/0001-Fix-buffer-overflow-in-ksba_oid_to_str.patch	1970-01-01 01:00:00.000000000 +0100
+++ libksba-1.3.1/debian/patches/0001-Fix-buffer-overflow-in-ksba_oid_to_str.patch	2014-11-25 17:52:39.000000000 +0100
@@ -0,0 +1,236 @@
+From f715b9e156dfa99ae829fc694e5a0abd23ef97d7 Mon Sep 17 00:00:00 2001
+From: Werner Koch <wk at gnupg.org>
+Date: Tue, 25 Nov 2014 11:47:28 +0100
+Subject: [PATCH] Fix buffer overflow in ksba_oid_to_str.
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+* src/oid.c (ksba_oid_to_str): Fix unsigned underflow.
+
+* tests/Makefile.am (noinst_PROGRAMS): Move t-oid to ..
+(TESTS): here.
+* tests/t-oid.c (test_oid_to_str): New.
+(main): Run the new tests by default.  The former functionality
+requires the use of one of the new options.
+--
+
+The code has an obvious error by not considering invalid encoding for
+arc-2.  A first byte of 0x80 can be used to make a value of less then
+80 and we then subtract 80 from that value as required by the OID
+encoding rules.  Due to the unsigned integer this results in a pretty
+long value which won't fit anymore into the allocated buffer.
+
+The fix is obvious.  Also added a few simple test cases.  Note that we
+keep on using sprintf instead of snprintf because managing the
+remaining length of the buffer would probably be more error prone than
+assuring that the buffer is large enough.  Getting rid of sprintf
+altogether by using direct conversion along with membuf_t like code
+might be possible.
+
+Reported-by: Hanno B??ck
+Signed-off-by: Werner Koch <wk at gnupg.org>
+---
+ src/oid.c          |  2 ++
+ tests/Makefile.am  |  4 +--
+ tests/t-dnparser.c |  2 +-
+ tests/t-oid.c      | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
+ 4 files changed, 99 insertions(+), 6 deletions(-)
+
+diff --git a/src/oid.c b/src/oid.c
+index d98f740..9061a4a 100644
+--- a/src/oid.c
++++ b/src/oid.c
+@@ -94,6 +94,8 @@ ksba_oid_to_str (const char *buffer, size_t length)
+         val <<= 7;
+         val |= buf[n] & 0x7f;
+       }
++    if (val < 80)
++      goto badoid;
+     val -= 80;
+     sprintf (p, "2.%lu", val);
+     p += strlen (p);
+diff --git a/tests/Makefile.am b/tests/Makefile.am
+index bb32172..759b626 100644
+--- a/tests/Makefile.am
++++ b/tests/Makefile.am
+@@ -39,13 +39,13 @@ EXTRA_DIST = $(test_certs) samples/README mkoidtbl.awk
+ BUILT_SOURCES = oidtranstbl.h
+ CLEANFILES = oidtranstbl.h
+ 
+-TESTS = cert-basic t-crl-parser t-dnparser
++TESTS = cert-basic t-crl-parser t-dnparser t-oid
+ 
+ AM_CFLAGS = $(GPG_ERROR_CFLAGS)
+ AM_LDFLAGS = -no-install
+ 
+ noinst_HEADERS = t-common.h
+-noinst_PROGRAMS = $(TESTS) t-cms-parser t-crl-parser t-dnparser t-ocsp t-oid
++noinst_PROGRAMS = $(TESTS) t-cms-parser t-crl-parser t-dnparser t-ocsp
+ LDADD = ../src/libksba.la $(GPG_ERROR_LIBS)
+ 
+ t_ocsp_SOURCES = t-ocsp.c sha1.c
+diff --git a/tests/t-dnparser.c b/tests/t-dnparser.c
+index c8d4b0d..ef4ab5d 100644
+--- a/tests/t-dnparser.c
++++ b/tests/t-dnparser.c
+@@ -143,7 +143,7 @@ main (int argc, char **argv)
+       if (!feof (stdin))
+         fail ("read error or input too large");
+ 
+-      fail ("no yet implemented");
++      fail ("not yet implemented");
+ 
+     }
+   else if (argc == 2 && !strcmp (argv[1], "--to-der") )
+diff --git a/tests/t-oid.c b/tests/t-oid.c
+index 95fc7f5..be68d52 100644
+--- a/tests/t-oid.c
++++ b/tests/t-oid.c
+@@ -27,6 +27,9 @@
+ 
+ #include "../src/ksba.h"
+ 
++#define PGM "t-oid"
++#define BADOID "1.3.6.1.4.1.11591.2.12242973"
++
+ 
+ static void *
+ read_into_buffer (FILE *fp, size_t *r_length)
+@@ -68,23 +71,104 @@ read_into_buffer (FILE *fp, size_t *r_length)
+ }
+ 
+ 
++static void
++test_oid_to_str (void)
++{
++  struct {
++    unsigned int binlen;
++    unsigned char *bin;
++    char *str;
++  } tests[] = {
++
++    {  7, "\x02\x82\x06\x01\x0A\x0C\x00",
++       "0.2.262.1.10.12.0"
++    },
++    {  7, "\x02\x82\x06\x01\x0A\x0C\x01",
++       "0.2.262.1.10.12.1"
++    },
++    {  7, "\x2A\x86\x48\xCE\x38\x04\x01",
++       "1.2.840.10040.4.1"
++    },
++    {  7, "\x2A\x86\x48\xCE\x38\x04\x03",
++       "1.2.840.10040.4.3"
++    },
++    { 10, "\x2B\x06\x01\x04\x01\xDA\x47\x02\x01\x01",
++      "1.3.6.1.4.1.11591.2.1.1"
++    },
++    {  3, "\x55\x1D\x0E",
++       "2.5.29.14"
++    },
++    {  9, "\x80\x02\x70\x50\x25\x46\xfd\x0c\xc0",
++       BADOID
++    },
++    {  1, "\x80",
++       BADOID
++    },
++    {  2, "\x81\x00",
++       "2.48"
++    },
++    {  2, "\x81\x01",
++       "2.49"
++    },
++    {  2, "\x81\x7f",
++       "2.175"
++    },
++    {  2, "\x81\x80",  /* legal encoding? */
++       "2.48"
++    },
++    {  2, "\x81\x81\x01",  /* legal encoding? */
++       "2.49"
++    },
++    {  0, "",
++       ""
++    },
++
++    { 0, NULL, NULL }
++  };
++  int tidx;
++  char *str;
++
++  for (tidx=0; tests[tidx].bin; tidx++)
++    {
++      str = ksba_oid_to_str (tests[tidx].bin, tests[tidx].binlen);
++      if (!str)
++        {
++          perror ("ksba_oid_to_str failed");
++          exit (1);
++        }
++      if (strcmp (tests[tidx].str, str))
++        {
++          fprintf (stderr, "ksba_oid_to_str test %d failed\n", tidx);
++          fprintf (stderr, "  got=%s\n", str);
++          fprintf (stderr, " want=%s\n", tests[tidx].str);
++          exit (1);
++        }
++    }
++}
++
+ 
+ int
+ main (int argc, char **argv)
+ {
+   gpg_error_t err;
++
+   if (argc)
+     {
+       argc--;
+       argv++;
+     }
+ 
+-  if (argc)
++
++  if (!argc)
++    {
++      test_oid_to_str ();
++    }
++  else if (!strcmp (*argv, "--from-str"))
+     {
+       unsigned char *buffer;
+       size_t n, buflen;
+ 
+-      for ( ;argc ; argc--, argv++)
++      for (argv++,argc-- ; argc; argc--, argv++)
+         {
+           err = ksba_oid_from_str (*argv, &buffer, &buflen);
+           if (err)
+@@ -100,18 +184,25 @@ main (int argc, char **argv)
+           free (buffer);
+         }
+     }
+-  else
++  else if (!strcmp (*argv, "--to-str"))
+     {
+       char *buffer;
+       size_t buflen;
+       char *result;
+ 
++      argv++;argc--;
++
+       buffer = read_into_buffer (stdin, &buflen);
+       result = ksba_oid_to_str (buffer, buflen);
+       free (buffer);
+       printf ("%s\n", result? result:"[malloc failed]");
+       free (result);
+     }
++  else
++    {
++      fputs ("usage: "PGM" [--from-str|--to-str]\n", stderr);
++      return 1;
++    }
+ 
+   return 0;
+ }
+-- 
+2.1.3
+
diff -Nru libksba-1.3.1/debian/patches/series libksba-1.3.1/debian/patches/series
--- libksba-1.3.1/debian/patches/series	1970-01-01 01:00:00.000000000 +0100
+++ libksba-1.3.1/debian/patches/series	2014-11-25 17:52:39.000000000 +0100
@@ -0,0 +1 @@
+0001-Fix-buffer-overflow-in-ksba_oid_to_str.patch


More information about the Pkg-gnutls-maint mailing list