Fix integer overflow in the BER decoder.
authorWerner Koch <wk@gnupg.org>
Thu, 9 Apr 2015 09:17:28 +0000 (11:17 +0200)
committerWerner Koch <wk@gnupg.org>
Thu, 9 Apr 2015 09:17:28 +0000 (11:17 +0200)
commitaea7b6032865740478ca4b706850a5217f1c3887
treedfee8070479a698fc925ea951a4c4962412ded27
parent243d12fdec66a4360fbb3e307a046b39b5b4ffc3
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@gnupg.org>
src/ber-decoder.c