From 5450329ad1778d72f117b68e5edb97ae1bf4d438 Mon Sep 17 00:00:00 2001 From: usa Date: Thu, 14 Sep 2017 11:41:59 +0000 Subject: [PATCH] asn1: fix out-of-bounds read in decoding constructed objects * OpenSSL::ASN1.{decode,decode_all,traverse}: have a bug of out-of-bounds read. int_ossl_asn1_decode0_cons() does not give the correct available length to ossl_asn1_decode() when decoding the inner components of a constructed object. This can cause out-of-bounds read if a crafted input given. Reference: https://hackerone.com/reports/170316 https://github.com/ruby/openssl/commit/1648afef33c1d97fb203c82291b8a61269e85d3b git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_2@59903 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 13 +++++++++++++ ext/openssl/ossl_asn1.c | 13 ++++++------- test/openssl/test_asn1.rb | 23 +++++++++++++++++++++++ 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7561c35eb705..6288f67500fd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,6 +17,19 @@ Thu Sep 14 20:44:26 2017 SHIBATA Hiroshi * ext/json: bump to version 1.8.1.1. [Backport #13853] + +Thu Sep 14 20:39:39 2017 Kazuki Yamaguchi + + asn1: fix out-of-bounds read in decoding constructed objects + + * OpenSSL::ASN1.{decode,decode_all,traverse}: have a bug of + out-of-bounds read. int_ossl_asn1_decode0_cons() does not give the + correct available length to ossl_asn1_decode() when decoding the + inner components of a constructed object. This can cause + out-of-bounds read if a crafted input given. + + Reference: https://hackerone.com/reports/170316 + https://github.com/ruby/openssl/commit/1648afef33c1d97fb203c82291b8a61269e85d3b Thu Sep 14 20:36:54 2017 Yusuke Endoh diff --git a/ext/openssl/ossl_asn1.c b/ext/openssl/ossl_asn1.c index 6d564a312f35..719063c551e5 100644 --- a/ext/openssl/ossl_asn1.c +++ b/ext/openssl/ossl_asn1.c @@ -871,19 +871,18 @@ int_ossl_asn1_decode0_cons(unsigned char **pp, long max_len, long length, { VALUE value, asn1data, ary; int infinite; - long off = *offset; + long available_len, off = *offset; infinite = (j == 0x21); ary = rb_ary_new(); - while (length > 0 || infinite) { + available_len = infinite ? max_len : length; + while (available_len > 0) { long inner_read = 0; - value = ossl_asn1_decode0(pp, max_len, &off, depth + 1, yield, &inner_read); + value = ossl_asn1_decode0(pp, available_len, &off, depth + 1, yield, &inner_read); *num_read += inner_read; - max_len -= inner_read; + available_len -= inner_read; rb_ary_push(ary, value); - if (length > 0) - length -= inner_read; if (infinite && NUM2INT(ossl_asn1_get_tag(value)) == V_ASN1_EOC && @@ -974,7 +973,7 @@ ossl_asn1_decode0(unsigned char **pp, long length, long *offset, int depth, if(j & V_ASN1_CONSTRUCTED) { *pp += hlen; off += hlen; - asn1data = int_ossl_asn1_decode0_cons(pp, length, len, &off, depth, yield, j, tag, tag_class, &inner_read); + asn1data = int_ossl_asn1_decode0_cons(pp, length - hlen, len, &off, depth, yield, j, tag, tag_class, &inner_read); inner_read += hlen; } else { diff --git a/test/openssl/test_asn1.rb b/test/openssl/test_asn1.rb index 9fb5a551c66d..a6d7c2c14e00 100644 --- a/test/openssl/test_asn1.rb +++ b/test/openssl/test_asn1.rb @@ -595,6 +595,29 @@ def test_recursive_octet_string_parse assert_equal(false, asn1.value[3].infinite_length) end + def test_decode_constructed_overread + test = %w{ 31 06 31 02 30 02 05 00 } + # ^ <- invalid + raw = [test.join].pack("H*") + ret = [] + assert_raise(OpenSSL::ASN1::ASN1Error) { + OpenSSL::ASN1.traverse(raw) { |x| ret << x } + } + assert_equal 2, ret.size + assert_equal 17, ret[0][6] + assert_equal 17, ret[1][6] + + test = %w{ 31 80 30 03 00 00 } + # ^ <- invalid + raw = [test.join].pack("H*") + ret = [] + assert_raise(OpenSSL::ASN1::ASN1Error) { + OpenSSL::ASN1.traverse(raw) { |x| ret << x } + } + assert_equal 1, ret.size + assert_equal 17, ret[0][6] + end + private def assert_universal(tag, asn1)