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