Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE:Leap:42.3:Staging:E
libksba
0002-Fix-integer-overflow-in-the-BER-decoder.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 0002-Fix-integer-overflow-in-the-BER-decoder.patch of Package libksba
From aea7b6032865740478ca4b706850a5217f1c3887 Mon Sep 17 00:00:00 2001 From: Werner Koch <wk@gnupg.org> Date: Thu, 9 Apr 2015 11:17:28 +0200 Subject: [PATCH 2/3] Fix integer overflow in the BER decoder. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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@gnupg.org> --- src/ber-decoder.c | 71 ++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 54 insertions(+), 17 deletions(-) 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++) -- 2.6.2
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
