[git] KSBA - branch, master, updated. libksba-1.3.2-8-g07116a3
by Werner Koch
cvs at cvs.gnupg.org
Thu Apr 9 11:52:22 CEST 2015
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "KSBA is a library to access X.509 certificates and CMS data.".
The branch, master has been updated
via 07116a314f4dcd4d96990bbd74db95a03a9f650a (commit)
via aea7b6032865740478ca4b706850a5217f1c3887 (commit)
via 243d12fdec66a4360fbb3e307a046b39b5b4ffc3 (commit)
from 792f4b36f998beba3515b776e8ca76ecbf20e468 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 07116a314f4dcd4d96990bbd74db95a03a9f650a
Author: Werner Koch <wk at gnupg.org>
Date: Thu Apr 9 11:50:03 2015 +0200
Do not abort on decoder stack overflow.
* src/ber-decoder.c (push_decoder_state, pop_decoder_state): Return an
error code.
(set_error): Prefix error message with "ksba:". Act on new return code.
(decoder_next): Act on new return code.
--
This changes the behaviour from
gpgsm: unknown hash algorithm '1.8.48.48.48.48.48.48.48.48'
gpgsm: detached signature w/o data - assuming certs-only
ERROR: decoder stack overflow!
Aborted
to
gpgsm: detached signature w/o data - assuming certs-only
ksba: ber-decoder: stack overflow!
gpgsm: ksba_cms_parse failed: Limit reached
Use "gpgsm --verify FILE" to exhibit the problem. FILE is
-----BEGIN PGP ARMORED FILE-----
MDAGCSqGSIb3DQEHAqCAMDACAQExDzANBgkwMDAwMDAwMDAwADCABgkwMDAwMDAw
MDAAMDEwoIGTMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw
MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw
MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw
MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw
MDAwMDAwMDAwMDAwMDAjMDA=
=PQdP
-----END PGP ARMORED FILE-----
Reported-by: Hanno Böck
Signed-off-by: Werner Koch <wk at gnupg.org>
diff --git a/src/ber-decoder.c b/src/ber-decoder.c
index b4689fa..9e70d92 100644
--- a/src/ber-decoder.c
+++ b/src/ber-decoder.c
@@ -1,5 +1,5 @@
/* ber-decoder.c - Basic Encoding Rules Decoder
- * Copyright (C) 2001, 2004, 2006, 2012 g10 Code GmbH
+ * Copyright (C) 2001, 2004, 2006, 2012, 2015 g10 Code GmbH
*
* This file is part of KSBA.
*
@@ -175,26 +175,28 @@ dump_decoder_state (DECODER_STATE ds)
}
/* Push ITEM onto the stack */
-static void
+static gpg_error_t
push_decoder_state (DECODER_STATE ds)
{
if (ds->idx >= ds->stacksize)
{
- fprintf (stderr, "ERROR: decoder stack overflow!\n");
- abort ();
+ fprintf (stderr, "ksba: ber-decoder: stack overflow!\n");
+ return gpg_error (GPG_ERR_LIMIT_REACHED);
}
ds->stack[ds->idx++] = ds->cur;
+ return 0;
}
-static void
+static gpg_error_t
pop_decoder_state (DECODER_STATE ds)
{
if (!ds->idx)
{
- fprintf (stderr, "ERROR: decoder stack underflow!\n");
- abort ();
+ fprintf (stderr, "ksba: ber-decoder: stack underflow!\n");
+ return gpg_error (GPG_ERR_INTERNAL);
}
ds->cur = ds->stack[--ds->idx];
+ return 0;
}
@@ -202,7 +204,7 @@ pop_decoder_state (DECODER_STATE ds)
static int
set_error (BerDecoder d, AsnNode node, const char *text)
{
- fprintf (stderr,"ber-decoder: node `%s': %s\n",
+ fprintf (stderr,"ksba: ber-decoder: node `%s': %s\n",
node? node->name:"?", text);
d->last_errdesc = text;
return gpg_error (GPG_ERR_BAD_BER);
@@ -955,9 +957,9 @@ decoder_next (BerDecoder d)
&& (ds->cur.nread
> ds->stack[ds->idx-1].length))
{
- fprintf (stderr, " ERROR: object length field "
+ fprintf (stderr, "ksba: ERROR: object length field "
"%d octects too large\n",
- ds->cur.nread > ds->cur.length);
+ ds->cur.nread - ds->cur.length);
ds->cur.nread = ds->cur.length;
}
if ( ds->idx
@@ -967,7 +969,9 @@ decoder_next (BerDecoder d)
>= ds->stack[ds->idx-1].length))))
{
int n = ds->cur.nread;
- pop_decoder_state (ds);
+ err = pop_decoder_state (ds);
+ if (err)
+ return err;
ds->cur.nread += n;
ds->cur.went_up++;
}
@@ -983,7 +987,9 @@ decoder_next (BerDecoder d)
/* prepare for the next level */
ds->cur.length = ti.length;
ds->cur.ndef_length = ti.ndef;
- push_decoder_state (ds);
+ err = push_decoder_state (ds);
+ if (err)
+ return err;
ds->cur.length = 0;
ds->cur.ndef_length = 0;
ds->cur.nread = 0;
commit aea7b6032865740478ca4b706850a5217f1c3887
Author: Werner Koch <wk at gnupg.org>
Date: Thu Apr 9 11:17:28 2015 +0200
Fix integer overflow in the BER decoder.
* src/ber-decoder.c (ber_decoder_s): Change val.length from int to
size_t.
(sum_a1_a2_gt_b, sum_a1_a2_ge_b): New.
(decoder_next): Check for integer overflow. Use new sum function for
size check.
(_ksba_ber_decoder_dump): Use size_t for n to match change of
val.length. Adjust printf fomrat. Check for integer overflow and use
gpg_error_from_syserror instead of GPG_ERR_ENOMEM.
(_ksba_ber_decoder_decode): Use new sum function for size check.
Check for integer overflow. Use size_t for n to match change of
val.length.
--
The actual bug described below is due to assigning an int
(val.length) to a size_t (ti.length). The int was too large and thus
negative so that the condition to check for too large objects didn't
worked. Changing the type would have been enough but other conditions
are possible. Thus the introduction of sum_a1_a2_ge_b for overflow
checking and checks when adding 100 extra bytes to malloc calls are
added.
Use "gpgsm --verify FILE" to exhibit the problem. FILE is
-----BEGIN PGP ARMORED FILE-----
MDAGCSqGSIb3DQEHAqCAMIACAQExDzANBgkwMDAwMDAwMDAwADAwBhcwMDAwMDAw
MDAwMDAwMDAwMDAwMDAwMAAwMTAGCTAwMDAwMDAwMDAwBgkwMDAwMDAwMDAwMAYJ
MDAwMDAwMDAwMDAXLDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw
MDAwMDAwMDAwCoYwMP////UwMDAwMDAwMDAwMDAwMDAwMA==
=tvju
-----END PGP ARMORED FILE-----
Without the patch this error occured:
gpgsm: unknown hash algorithm '1.8.48.48.48.48.48.48.48.48'
gpgsm: detached signature w/o data - assuming certs-only
=================================================================
==14322==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x60b00000aded at pc 0x462ca8 bp 0x7fffd5928d90 sp 0x7fffd5928d80
WRITE of size 1 at 0x60b00000aded thread T0
#0 0x462ca7 in base64_reader_cb [...]-2.1.2/sm/base64.c:363
#1 0x7f35e70b6365 (/usr/lib64/libksba.so.8+0x7365)
#2 0x7f35e70bee11 (/usr/lib64/libksba.so.8+0xfe11)
#3 0x7f35e70c75ed (/usr/lib64/libksba.so.8+0x185ed)
#4 0x7f35e70c7a9d (/usr/lib64/libksba.so.8+0x18a9d)
#5 0x7f35e70c356f (/usr/lib64/libksba.so.8+0x1456f)
#6 0x7f35e70c58bf (/usr/lib64/libksba.so.8+0x168bf)
#7 0x48cbee in gpgsm_verify [...]/gnupg-2.1.2/sm/verify.c:171
#8 0x412901 in main /data/gnupg/gnupg-2.1.2/sm/gpgsm.c:1795
#9 0x7f35e68d5f9f in __libc_start_main ([...]
#10 0x415a91 (/data/gnupg/gnupg-2.1.2/sm/gpgsm+0x415a91)
0x60b00000aded is located 0 bytes to the right of 109-byte region
[0x60b00000ad80,0x60b00000aded)
allocated by thread T0 here:
#0 0x7f35e782e6f7 in malloc [...]
#1 0x7f35e75040b0 (/usr/lib64/libgcrypt.so.20+0xc0b0)
SUMMARY: AddressSanitizer: heap-buffer-overflow [...]
Shadow bytes around the buggy address:
0x0c167fff9560: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c167fff9570: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c167fff9580: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c167fff9590: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c167fff95a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c167fff95b0: 00 00 00 00 00 00 00 00 00 00 00 00 00[05]fa fa
0x0c167fff95c0: fa fa fa fa fa fa 00 00 00 00 00 00 00 00 00 00
0x0c167fff95d0: 00 00 00 fa fa fa fa fa fa fa fa fa 00 00 00 00
Reported-by: Hanno Böck
Signed-off-by: Werner Koch <wk at gnupg.org>
diff --git a/src/ber-decoder.c b/src/ber-decoder.c
index 873f810..b4689fa 100644
--- a/src/ber-decoder.c
+++ b/src/ber-decoder.c
@@ -100,7 +100,7 @@ struct ber_decoder_s
struct
{
int primitive; /* current value is a primitive one */
- int length; /* length of the primitive one */
+ size_t length; /* length of the primitive one */
int nhdr; /* length of the header */
int tag;
int is_endtag;
@@ -109,6 +109,23 @@ struct ber_decoder_s
};
+
+/* Evaluate with overflow check: A1 + A2 > B */
+static inline int
+sum_a1_a2_gt_b (size_t a1, size_t a2, size_t b)
+{
+ size_t sum = a1 + a2;
+ return (sum < a1 || sum > b);
+}
+
+/* Evaluate with overflow check: A1 + A2 >= B */
+static inline int
+sum_a1_a2_ge_b (size_t a1, size_t a2, size_t b)
+{
+ size_t sum = a1 + a2;
+ return (sum < a1 || sum >= b);
+}
+
static DECODER_STATE
@@ -839,14 +856,16 @@ decoder_next (BerDecoder d)
{
/* We need some extra bytes to store the stuff we read ahead
at the end of the module which is later pushed back. */
- d->image.length = ti.length + 100;
d->image.used = 0;
+ d->image.length = ti.length + 100;
+ if (d->image.length < ti.length)
+ return gpg_error (GPG_ERR_BAD_BER);
d->image.buf = xtrymalloc (d->image.length);
if (!d->image.buf)
return gpg_error (GPG_ERR_ENOMEM);
}
- if (ti.nhdr + d->image.used >= d->image.length)
+ if (sum_a1_a2_ge_b (ti.nhdr, d->image.used, d->image.length))
return set_error (d, NULL, "image buffer too short to store the tag");
memcpy (d->image.buf + d->image.used, ti.buf, ti.nhdr);
@@ -1041,7 +1060,7 @@ _ksba_ber_decoder_dump (BerDecoder d, FILE *fp)
int depth = 0;
AsnNode node;
unsigned char *buf = NULL;
- size_t buflen = 0;;
+ size_t buflen = 0;
if (!d)
return gpg_error (GPG_ERR_INV_VALUE);
@@ -1063,9 +1082,9 @@ _ksba_ber_decoder_dump (BerDecoder d, FILE *fp)
if (node)
depth = distance (d->root, node);
- fprintf (fp, "%4lu %4u:%*s",
+ fprintf (fp, "%4lu %4lu:%*s",
ksba_reader_tell (d->reader) - d->val.nhdr,
- d->val.length,
+ (unsigned long)d->val.length,
depth*2, "");
if (node)
_ksba_asn_node_dump (node, fp);
@@ -1074,16 +1093,22 @@ _ksba_ber_decoder_dump (BerDecoder d, FILE *fp)
if (node && d->val.primitive)
{
- int i, n, c;
+ size_t n;
+ int i, c;
char *p;
if (!buf || buflen < d->val.length)
{
xfree (buf);
buflen = d->val.length + 100;
- buf = xtrymalloc (buflen);
- if (!buf)
- err = gpg_error (GPG_ERR_ENOMEM);
+ if (buflen < d->val.length)
+ err = gpg_error (GPG_ERR_BAD_BER); /* Overflow */
+ else
+ {
+ buf = xtrymalloc (buflen);
+ if (!buf)
+ err = gpg_error_from_syserror ();
+ }
}
for (n=0; !err && n < d->val.length; n++)
@@ -1171,8 +1196,6 @@ _ksba_ber_decoder_decode (BerDecoder d, const char *start_name,
while (!(err = decoder_next (d)))
{
- int n, c;
-
node = d->val.node;
/* Fixme: USE_IMAGE is only not used with the ber-dump utility
and thus of no big use. We should remove the other code
@@ -1188,7 +1211,7 @@ _ksba_ber_decoder_decode (BerDecoder d, const char *start_name,
if (node->type == TYPE_ANY)
node->actual_type = d->val.tag;
}
- if (d->image.used + d->val.length > d->image.length)
+ if (sum_a1_a2_gt_b (d->image.used, d->val.length, d->image.length))
err = set_error(d, NULL, "TLV length too large");
else if (d->val.primitive)
{
@@ -1196,18 +1219,32 @@ _ksba_ber_decoder_decode (BerDecoder d, const char *start_name,
d->image.buf + d->image.used, d->val.length))
err = eof_or_error (d, 1);
else
- d->image.used += d->val.length;
+ {
+ size_t sum = d->image.used + d->val.length;
+ if (sum < d->image.used)
+ err = gpg_error (GPG_ERR_BAD_BER);
+ else
+ d->image.used = sum;
+ }
}
}
else if (node && d->val.primitive)
{
+ size_t n;
+ int c;
+
if (!buf || buflen < d->val.length)
{
xfree (buf);
buflen = d->val.length + 100;
- buf = xtrymalloc (buflen);
- if (!buf)
- err = gpg_error (GPG_ERR_ENOMEM);
+ if (buflen < d->val.length)
+ err = gpg_error (GPG_ERR_BAD_BER);
+ else
+ {
+ buf = xtrymalloc (buflen);
+ if (!buf)
+ err = gpg_error_from_syserror ();
+ }
}
for (n=0; !err && n < d->val.length; n++)
commit 243d12fdec66a4360fbb3e307a046b39b5b4ffc3
Author: Werner Koch <wk at gnupg.org>
Date: Wed Apr 8 18:51:21 2015 +0200
Fix encoding of invalid utf-8 strings in dn.c
* 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 at gnupg.org>
diff --git a/src/dn.c b/src/dn.c
index 4fab689..d207bf0 100644
--- a/src/dn.c
+++ b/src/dn.c
@@ -260,7 +260,7 @@ append_quoted (struct stringbuf *sb, const unsigned char *value, size_t length,
n += skip;
if ( *s < ' ' || *s > 126 )
{
- sprintf (tmp, "\\%02X", *s);
+ snprintf (tmp, sizeof tmp, "\\%02X", *s);
put_stringbuf_mem (sb, tmp, 3);
}
else
@@ -300,7 +300,6 @@ append_utf8_value (const unsigned char *value, size_t length,
length--;
}
- /* FIXME: check that the invalid encoding handling is correct */
for (s=value, n=0;;)
{
for (value = s; n < length && !(*s & 0x80); n++, s++)
@@ -309,8 +308,9 @@ append_utf8_value (const unsigned char *value, size_t length,
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;
@@ -320,21 +320,31 @@ append_utf8_value (const unsigned char *value, size_t length,
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
+ {
+ if (n+nmore > length)
+ nmore = length - n; /* Oops, encoding to short */
+
+ tmp[0] = *s++; n++;
+ for (i=1; 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);
}
}
@@ -618,7 +628,7 @@ append_atv (const unsigned char *image, AsnNode root, struct stringbuf *sb)
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;
-----------------------------------------------------------------------
Summary of changes:
src/ber-decoder.c | 101 ++++++++++++++++++++++++++++++++++++++----------------
src/dn.c | 44 +++++++++++++++---------
2 files changed, 99 insertions(+), 46 deletions(-)
hooks/post-receive
--
KSBA is a library to access X.509 certificates and CMS data.
http://git.gnupg.org
More information about the Gnupg-commits
mailing list