[svn] ksba - r307 - in trunk: . src tests
svn author wk
cvs at cvs.gnupg.org
Mon Jun 29 15:40:07 CEST 2009
Author: wk
Date: 2009-06-29 15:40:07 +0200 (Mon, 29 Jun 2009)
New Revision: 307
Added:
trunk/tests/t-oid.c
Modified:
trunk/NEWS
trunk/src/ChangeLog
trunk/src/asn1-func.c
trunk/src/oid.c
Log:
Detect overflow while parsing OIDs.
Modified: trunk/src/ChangeLog
===================================================================
--- trunk/src/ChangeLog 2009-06-05 10:29:33 UTC (rev 306)
+++ trunk/src/ChangeLog 2009-06-29 13:40:07 UTC (rev 307)
@@ -1,3 +1,13 @@
+2009-06-29 Werner Koch <wk at g10code.com>
+
+ * oid.c (ksba_oid_to_str): Add an overflow check so that we can
+ detect bogus OIDs.
+
+ * asn1-func.c (copy_value): Fix out-of-bounds assignment of a
+ boolean to HELPBUF. Due to alignment rules this was not
+ exploitable and we did not even used this code path. Reported by
+ David Binderman.
+
2009-05-28 Werner Koch <wk at g10code.com>
* der-encoder.c (_ksba_der_store_null, sum_up_lengths): Actually
Modified: trunk/NEWS
===================================================================
--- trunk/NEWS 2009-06-05 10:29:33 UTC (rev 306)
+++ trunk/NEWS 2009-06-29 13:40:07 UTC (rev 307)
@@ -1,7 +1,9 @@
Noteworthy changes in version 1.0.7
------------------------------------------------
+ * Detect overflow while parsing OIDs.
+
Noteworthy changes in version 1.0.6 (2009-06-05)
------------------------------------------------
Modified: trunk/src/asn1-func.c
===================================================================
--- trunk/src/asn1-func.c 2009-06-05 10:29:33 UTC (rev 306)
+++ trunk/src/asn1-func.c 2009-06-29 13:40:07 UTC (rev 307)
@@ -171,7 +171,7 @@
break;
case VALTYPE_BOOL:
len = 1;
- helpbuf[1] = s->value.v_bool;
+ helpbuf[0] = s->value.v_bool;
buf = helpbuf;
break;
case VALTYPE_CSTR:
Modified: trunk/src/oid.c
===================================================================
--- trunk/src/oid.c 2009-06-05 10:29:33 UTC (rev 306)
+++ trunk/src/oid.c 2009-06-29 13:40:07 UTC (rev 307)
@@ -1,5 +1,5 @@
/* oid.c - Object identifier helper functions
- * Copyright (C) 2001 g10 Code GmbH
+ * Copyright (C) 2001, 2009 g10 Code GmbH
*
* This file is part of KSBA.
*
@@ -54,8 +54,10 @@
const unsigned char *buf = buffer;
char *string, *p;
int n = 0;
- unsigned long val;
+ unsigned long val, valmask;
+ valmask = (unsigned long)0xfe << (8 * (sizeof (valmask) - 1));
+
/* To calculate the length of the string we can safely assume an
upper limit of 3 decimal characters per byte. Two extra bytes
account for the special first octect */
@@ -68,9 +70,6 @@
return string;
}
- /* fixme: open code the sprintf so that we can cope with arbitrary
- long integers - at least we should check for overflow of ulong */
-
if (buf[0] < 40)
p += sprintf (p, "0.%d", buf[n]);
else if (buf[0] < 80)
@@ -79,6 +78,8 @@
val = buf[n] & 0x7f;
while ( (buf[n]&0x80) && ++n < length )
{
+ if ( (val & valmask) )
+ goto badoid; /* Overflow. */
val <<= 7;
val |= buf[n] & 0x7f;
}
@@ -91,6 +92,8 @@
val = buf[n] & 0x7f;
while ( (buf[n]&0x80) && ++n < length )
{
+ if ( (val & valmask) )
+ goto badoid; /* Overflow. */
val <<= 7;
val |= buf[n] & 0x7f;
}
@@ -100,6 +103,15 @@
*p = 0;
return string;
+
+ badoid:
+ /* Return a special OID (gnu.gnupg.badoid) to indicate the error
+ case. The OID is broken and thus we return one which can't do
+ any harm. Formally this does not need to be a bad OID but an OID
+ with an arc that can't be represented in a 32 bit word is more
+ than likely corrupt. */
+ xfree (string);
+ return xtrystrdup ("1.3.6.1.4.1.11591.2.12242973");
}
Added: trunk/tests/t-oid.c
===================================================================
--- trunk/tests/t-oid.c (rev 0)
+++ trunk/tests/t-oid.c 2009-06-29 13:40:07 UTC (rev 307)
@@ -0,0 +1,89 @@
+/* t-oid.c - Test utility for the OID functions
+ * Copyright (C) 2009 g10 Code GmbH
+ *
+ * This file is part of KSBA.
+ *
+ * KSBA is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * KSBA is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <assert.h>
+#include <time.h>
+#include <errno.h>
+
+#include "../src/ksba.h"
+
+
+static void *
+read_into_buffer (FILE *fp, size_t *r_length)
+{
+ char *buffer;
+ size_t buflen;
+ size_t nread, bufsize = 0;
+
+ *r_length = 0;
+#define NCHUNK 8192
+#ifdef HAVE_W32_SYSTEM
+ setmode (fileno(fp), O_BINARY);
+#endif
+ buffer = NULL;
+ buflen = 0;
+ do
+ {
+ bufsize += NCHUNK;
+ buffer = realloc (buffer, bufsize);
+ if (!buffer)
+ {
+ perror ("realloc failed");
+ exit (1);
+ }
+
+ nread = fread (buffer + buflen, 1, NCHUNK, fp);
+ if (nread < NCHUNK && ferror (fp))
+ {
+ perror ("fread failed");
+ exit (1);
+ }
+ buflen += nread;
+ }
+ while (nread == NCHUNK);
+#undef NCHUNK
+
+ *r_length = buflen;
+ return buffer;
+}
+
+
+
+int
+main (int argc, char **argv)
+{
+ char *buffer;
+ size_t buflen;
+ char *result;
+
+ (void)argc;
+ (void)argv;
+
+ buffer = read_into_buffer (stdin, &buflen);
+ result = ksba_oid_to_str (buffer, buflen);
+ free (buffer);
+ printf ("%s\n", result? result:"[malloc failed]");
+ free (result);
+
+ return 0;
+}
More information about the Gnupg-commits
mailing list