From ea5e22d102a357114846874c2ff641f4a18dbdf1 Mon Sep 17 00:00:00 2001 From: nagachika Date: Fri, 26 Aug 2016 18:48:55 +0000 Subject: backport fix memory leak from upstream. https://github.com/ruby/openssl/compare/3a2840e80d275895523a526ce56e4f6e7b8f9cc4...1e30cd395b14ef46e04bdd9ab72f10067890b265 patches are provided by rhe (Kazuki Yamaguchi). * ext/openssl/ossl_config.c: fix memory leak. [ruby-core:76922] [Bug #12680] * ext/openssl/ossl_ocsp.c: ditto. * ext/openssl/ossl_pkcs12.c: ditto. * ext/openssl/ossl_pkcs7.c: ditto. * ext/openssl/ossl_pkey_ec.c: ditto. * ext/openssl/ossl_x509.h: ditto. * ext/openssl/ossl_x509attr.c: ditto. * ext/openssl/ossl_x509crl.c: ditto. * ext/openssl/ossl_x509ext.c: ditto. * ext/openssl/ossl_x509req.c: ditto. * ext/openssl/ossl_x509revoked.c: ditto. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_3@56018 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 25 ++++++++++++++++++ ext/openssl/ossl_config.c | 11 ++++---- ext/openssl/ossl_config.h | 1 - ext/openssl/ossl_ocsp.c | 11 +++++--- ext/openssl/ossl_pkcs12.c | 2 +- ext/openssl/ossl_pkcs7.c | 12 ++++----- ext/openssl/ossl_pkey_ec.c | 24 ++++++++--------- ext/openssl/ossl_x509.h | 2 +- ext/openssl/ossl_x509attr.c | 20 +++++++------- ext/openssl/ossl_x509crl.c | 6 +++-- ext/openssl/ossl_x509ext.c | 59 ++++++++++++------------------------------ ext/openssl/ossl_x509req.c | 4 +-- ext/openssl/ossl_x509revoked.c | 4 +-- version.h | 6 ++--- 14 files changed, 96 insertions(+), 91 deletions(-) diff --git a/ChangeLog b/ChangeLog index c9c74c8d44..d5ac932c2e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,28 @@ +Sat Aug 27 03:37:49 2016 Kazuki Yamaguchi + + * ext/openssl/ossl_config.c: fix memory leak. + [ruby-core:76922] [Bug #12680] + + * ext/openssl/ossl_ocsp.c: ditto. + + * ext/openssl/ossl_pkcs12.c: ditto. + + * ext/openssl/ossl_pkcs7.c: ditto. + + * ext/openssl/ossl_pkey_ec.c: ditto. + + * ext/openssl/ossl_x509.h: ditto. + + * ext/openssl/ossl_x509attr.c: ditto. + + * ext/openssl/ossl_x509crl.c: ditto. + + * ext/openssl/ossl_x509ext.c: ditto. + + * ext/openssl/ossl_x509req.c: ditto. + + * ext/openssl/ossl_x509revoked.c: ditto. + Thu Aug 25 00:19:24 2016 SHIBATA Hiroshi * lib/rubygems/specification.rb: `coding` is affect only first line except diff --git a/ext/openssl/ossl_config.c b/ext/openssl/ossl_config.c index 4e00fbe4ad..47d2658453 100644 --- a/ext/openssl/ossl_config.c +++ b/ext/openssl/ossl_config.c @@ -26,13 +26,13 @@ VALUE eConfigError; */ /* - * GetConfigPtr is a public C-level function for getting OpenSSL CONF struct + * DupConfigPtr is a public C-level function for getting OpenSSL CONF struct * from an OpenSSL::Config(eConfig) instance. We decided to implement * OpenSSL::Config in Ruby level but we need to pass native CONF struct for * some OpenSSL features such as X509V3_EXT_*. */ CONF * -GetConfigPtr(VALUE obj) +DupConfigPtr(VALUE obj) { CONF *conf; VALUE str; @@ -50,9 +50,10 @@ GetConfigPtr(VALUE obj) if(!NCONF_load_bio(conf, bio, &eline)){ BIO_free(bio); NCONF_free(conf); - if (eline <= 0) ossl_raise(eConfigError, "wrong config format"); - else ossl_raise(eConfigError, "error in line %d", eline); - ossl_raise(eConfigError, NULL); + if (eline <= 0) + ossl_raise(eConfigError, "wrong config format"); + else + ossl_raise(eConfigError, "error in line %d", eline); } BIO_free(bio); diff --git a/ext/openssl/ossl_config.h b/ext/openssl/ossl_config.h index 077e2f7400..627d297ba3 100644 --- a/ext/openssl/ossl_config.h +++ b/ext/openssl/ossl_config.h @@ -13,7 +13,6 @@ extern VALUE cConfig; extern VALUE eConfigError; -CONF* GetConfigPtr(VALUE obj); CONF* DupConfigPtr(VALUE obj); void Init_ossl_config(void); diff --git a/ext/openssl/ossl_ocsp.c b/ext/openssl/ossl_ocsp.c index 02b67429e6..b97e26cf92 100644 --- a/ext/openssl/ossl_ocsp.c +++ b/ext/openssl/ossl_ocsp.c @@ -271,12 +271,17 @@ static VALUE ossl_ocspreq_add_certid(VALUE self, VALUE certid) { OCSP_REQUEST *req; - OCSP_CERTID *id; + OCSP_CERTID *id, *id_new; GetOCSPReq(self, req); GetOCSPCertId(certid, id); - if(!OCSP_request_add0_id(req, OCSP_CERTID_dup(id))) - ossl_raise(eOCSPError, NULL); + + if (!(id_new = OCSP_CERTID_dup(id))) + ossl_raise(eOCSPError, "OCSP_CERTID_dup"); + if (!OCSP_request_add0_id(req, id_new)) { + OCSP_CERTID_free(id_new); + ossl_raise(eOCSPError, "OCSP_request_add0_id"); + } return self; } diff --git a/ext/openssl/ossl_pkcs12.c b/ext/openssl/ossl_pkcs12.c index e5052d47ea..c70ebca195 100644 --- a/ext/openssl/ossl_pkcs12.c +++ b/ext/openssl/ossl_pkcs12.c @@ -104,7 +104,6 @@ ossl_pkcs12_s_create(int argc, VALUE *argv, VALUE self) friendlyname = NIL_P(name) ? NULL : StringValuePtr(name); key = GetPKeyPtr(pkey); x509 = GetX509CertPtr(cert); - x509s = NIL_P(ca) ? NULL : ossl_x509_ary2sk(ca); /* TODO: make a VALUE to nid function */ if (!NIL_P(key_nid)) { if ((nkey = OBJ_txt2nid(StringValuePtr(key_nid))) == NID_undef) @@ -122,6 +121,7 @@ ossl_pkcs12_s_create(int argc, VALUE *argv, VALUE self) ktype = NUM2INT(keytype); obj = NewPKCS12(cPKCS12); + x509s = NIL_P(ca) ? NULL : ossl_x509_ary2sk(ca); p12 = PKCS12_create(passphrase, friendlyname, key, x509, x509s, nkey, ncert, kiter, miter, ktype); sk_X509_pop_free(x509s, X509_free); diff --git a/ext/openssl/ossl_pkcs7.c b/ext/openssl/ossl_pkcs7.c index 9ca3abd764..04e41db598 100644 --- a/ext/openssl/ossl_pkcs7.c +++ b/ext/openssl/ossl_pkcs7.c @@ -755,7 +755,9 @@ ossl_pkcs7_verify(int argc, VALUE *argv, VALUE self) VALUE data; const char *msg; + GetPKCS7(self, p7); rb_scan_args(argc, argv, "22", &certs, &store, &indata, &flags); + x509st = GetX509StorePtr(store); flg = NIL_P(flags) ? 0 : NUM2INT(flags); if(NIL_P(indata)) indata = ossl_pkcs7_get_data(self); in = NIL_P(indata) ? NULL : ossl_obj2bio(indata); @@ -767,8 +769,6 @@ ossl_pkcs7_verify(int argc, VALUE *argv, VALUE self) rb_jump_tag(status); } } - x509st = GetX509StorePtr(store); - GetPKCS7(self, p7); if(!(out = BIO_new(BIO_s_mem()))){ BIO_free(in); sk_X509_pop_free(x509s, X509_free); @@ -776,13 +776,13 @@ ossl_pkcs7_verify(int argc, VALUE *argv, VALUE self) } ok = PKCS7_verify(p7, x509s, x509st, in, out, flg); BIO_free(in); - if (ok < 0) ossl_raise(ePKCS7Error, NULL); + sk_X509_pop_free(x509s, X509_free); + if (ok < 0) ossl_raise(ePKCS7Error, "PKCS7_verify"); msg = ERR_reason_error_string(ERR_get_error()); ossl_pkcs7_set_err_string(self, msg ? rb_str_new2(msg) : Qnil); ERR_clear_error(); data = ossl_membio2str(out); ossl_pkcs7_set_data(self, data); - sk_X509_pop_free(x509s, X509_free); return (ok == 1) ? Qtrue : Qfalse; } @@ -822,12 +822,12 @@ ossl_pkcs7_add_data(VALUE self, VALUE data) char buf[4096]; int len; - in = ossl_obj2bio(data); GetPKCS7(self, pkcs7); if(PKCS7_type_is_signed(pkcs7)){ if(!PKCS7_content_new(pkcs7, NID_pkcs7_data)) ossl_raise(ePKCS7Error, NULL); } + in = ossl_obj2bio(data); if(!(out = PKCS7_dataInit(pkcs7, NULL))) goto err; for(;;){ if((len = BIO_read(in, buf, sizeof(buf))) <= 0) @@ -839,7 +839,7 @@ ossl_pkcs7_add_data(VALUE self, VALUE data) ossl_pkcs7_set_data(self, Qnil); err: - BIO_free(out); + BIO_free_all(out); BIO_free(in); if(ERR_peek_error()){ ossl_raise(ePKCS7Error, NULL); diff --git a/ext/openssl/ossl_pkey_ec.c b/ext/openssl/ossl_pkey_ec.c index c93e3cfb99..09987e5426 100644 --- a/ext/openssl/ossl_pkey_ec.c +++ b/ext/openssl/ossl_pkey_ec.c @@ -475,6 +475,7 @@ static VALUE ossl_ec_key_to_string(VALUE self, VALUE ciph, VALUE pass, int forma int private = 0; char *password = NULL; VALUE str; + const EVP_CIPHER *cipher = NULL; Require_EC_KEY(self, ec); @@ -487,25 +488,22 @@ static VALUE ossl_ec_key_to_string(VALUE self, VALUE ciph, VALUE pass, int forma if (EC_KEY_get0_private_key(ec)) private = 1; + if (!NIL_P(ciph)) { + cipher = GetCipherPtr(ciph); + if (!NIL_P(pass)) { + StringValue(pass); + if (RSTRING_LENINT(pass) < OSSL_MIN_PWD_LEN) + ossl_raise(eOSSLError, "OpenSSL requires passwords to be at least four characters long"); + password = RSTRING_PTR(pass); + } + } + if (!(out = BIO_new(BIO_s_mem()))) ossl_raise(eECError, "BIO_new(BIO_s_mem())"); switch(format) { case EXPORT_PEM: if (private) { - const EVP_CIPHER *cipher; - if (!NIL_P(ciph)) { - cipher = GetCipherPtr(ciph); - if (!NIL_P(pass)) { - StringValue(pass); - if (RSTRING_LENINT(pass) < OSSL_MIN_PWD_LEN) - ossl_raise(eOSSLError, "OpenSSL requires passwords to be at least four characters long"); - password = RSTRING_PTR(pass); - } - } - else { - cipher = NULL; - } i = PEM_write_bio_ECPrivateKey(out, ec, cipher, NULL, 0, NULL, password); } else { i = PEM_write_bio_EC_PUBKEY(out, ec); diff --git a/ext/openssl/ossl_x509.h b/ext/openssl/ossl_x509.h index 8e9b233098..9dedb9a49a 100644 --- a/ext/openssl/ossl_x509.h +++ b/ext/openssl/ossl_x509.h @@ -24,7 +24,7 @@ extern VALUE cX509Attr; extern VALUE eX509AttrError; VALUE ossl_x509attr_new(X509_ATTRIBUTE *); -X509_ATTRIBUTE *DupX509AttrPtr(VALUE); +X509_ATTRIBUTE *GetX509AttrPtr(VALUE); void Init_ossl_x509attr(void); /* diff --git a/ext/openssl/ossl_x509attr.c b/ext/openssl/ossl_x509attr.c index d0f41c6bb8..b5a2441807 100644 --- a/ext/openssl/ossl_x509attr.c +++ b/ext/openssl/ossl_x509attr.c @@ -72,16 +72,13 @@ ossl_x509attr_new(X509_ATTRIBUTE *attr) } X509_ATTRIBUTE * -DupX509AttrPtr(VALUE obj) +GetX509AttrPtr(VALUE obj) { - X509_ATTRIBUTE *attr, *new; + X509_ATTRIBUTE *attr; SafeGetX509Attr(obj, attr); - if (!(new = X509_ATTRIBUTE_dup(attr))) { - ossl_raise(eX509AttrError, NULL); - } - return new; + return attr; } /* @@ -141,12 +138,15 @@ ossl_x509attr_set_oid(VALUE self, VALUE oid) ASN1_OBJECT *obj; char *s; - s = StringValuePtr(oid); + GetX509Attr(self, attr); + s = StringValueCStr(oid); obj = OBJ_txt2obj(s, 0); - if(!obj) obj = OBJ_txt2obj(s, 1); if(!obj) ossl_raise(eX509AttrError, NULL); - GetX509Attr(self, attr); - X509_ATTRIBUTE_set1_object(attr, obj); + if (!X509_ATTRIBUTE_set1_object(attr, obj)) { + ASN1_OBJECT_free(obj); + ossl_raise(eX509AttrError, "X509_ATTRIBUTE_set1_object"); + } + ASN1_OBJECT_free(obj); return oid; } diff --git a/ext/openssl/ossl_x509crl.c b/ext/openssl/ossl_x509crl.c index f64712efcd..3905762d3c 100644 --- a/ext/openssl/ossl_x509crl.c +++ b/ext/openssl/ossl_x509crl.c @@ -315,7 +315,8 @@ ossl_x509crl_set_revoked(VALUE self, VALUE ary) for (i=0; ireq_info->attributes = NULL; for (i=0;iextensions = NULL; for (i=0; i