bn: keep reference to temporary OpenSSL::BN object created by GetBNPtr()

GetBNPtr() accepts both OpenSSL::BN and Ruby integers. In the latter case, it creates a temporary OpenSSL::BN internally. The OpenSSL::BN object immediately disappears from the stack and is not protected from GC. Fixes: #87
ruby · Dec 4, 2016 · 182604c · 182604c
1 parent 72126d6
commit 182604c
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 19 deletions.
diff --git a/ext/openssl/ossl_bn.c b/ext/openssl/ossl_bn.c
@@ -120,30 +120,34 @@ integer_to_bnptr(VALUE obj, BIGNUM *orig)
     return bn;
 }
 
-static BIGNUM *
-try_convert_to_bnptr(VALUE obj)
+static VALUE
+try_convert_to_bn(VALUE obj)
 {
-    BIGNUM *bn = NULL;
-    VALUE newobj;
+    BIGNUM *bn;
+    VALUE newobj = Qnil;
 
-    if (rb_obj_is_kind_of(obj, cBN)) {
-	GetBN(obj, bn);
-    }
-    else if (RB_INTEGER_TYPE_P(obj)) {
+    if (rb_obj_is_kind_of(obj, cBN))
+	return obj;
+    if (RB_INTEGER_TYPE_P(obj)) {
 	newobj = NewBN(cBN); /* Handle potencial mem leaks */
 	bn = integer_to_bnptr(obj, NULL);
 	SetBN(newobj, bn);
     }
 
-    return bn;
+    return newobj;
 }
 
 BIGNUM *
-GetBNPtr(VALUE obj)
+ossl_bn_value_ptr(volatile VALUE *ptr)
 {
-    BIGNUM *bn = try_convert_to_bnptr(obj);
-    if (!bn)
+    VALUE tmp;
+    BIGNUM *bn;
+
+    tmp = try_convert_to_bn(*ptr);
+    if (NIL_P(tmp))
 	ossl_raise(rb_eTypeError, "Cannot convert into OpenSSL::BN");
+    GetBN(tmp, bn);
+    *ptr = tmp;
 
     return bn;
 }
@@ -893,10 +897,12 @@ ossl_bn_eq(VALUE self, VALUE other)
     BIGNUM *bn1, *bn2;
 
     GetBN(self, bn1);
-    /* BNPtr may raise, so we can't use here */
-    bn2 = try_convert_to_bnptr(other);
+    other = try_convert_to_bn(other);
+    if (NIL_P(other))
+	return Qfalse;
+    GetBN(other, bn2);
 
-    if (bn2 && !BN_cmp(bn1, bn2)) {
+    if (!BN_cmp(bn1, bn2)) {
 	return Qtrue;
     }
     return Qfalse;

diff --git a/ext/openssl/ossl_bn.h b/ext/openssl/ossl_bn.h
@@ -15,8 +15,10 @@ extern VALUE eBNError;
 
 extern BN_CTX *ossl_bn_ctx;
 
+#define GetBNPtr(obj) ossl_bn_value_ptr(&(obj))
+
 VALUE ossl_bn_new(const BIGNUM *);
-BIGNUM *GetBNPtr(VALUE);
+BIGNUM *ossl_bn_value_ptr(volatile VALUE *);
 void Init_ossl_bn(void);
 
 

diff --git a/ext/openssl/ossl_pkey_ec.c b/ext/openssl/ossl_pkey_ec.c
@@ -1635,7 +1635,7 @@ static VALUE ossl_ec_point_mul(int argc, VALUE *argv, VALUE self)
 	 * points  | self    | arg2[0] | arg2[1] | ...
 	 */
 	long i, num;
-	VALUE tmp_p, tmp_b;
+	VALUE bns_tmp, tmp_p, tmp_b;
 	const EC_POINT **points;
 	const BIGNUM **bignums;
 
@@ -1645,9 +1645,13 @@ static VALUE ossl_ec_point_mul(int argc, VALUE *argv, VALUE self)
 	    ossl_raise(rb_eArgError, "bns must be 1 longer than points; see the documentation");
 
 	num = RARRAY_LEN(arg1);
+	bns_tmp = rb_ary_tmp_new(num);
 	bignums = ALLOCV_N(const BIGNUM *, tmp_b, num);
-	for (i = 0; i < num; i++)
-	    bignums[i] = GetBNPtr(RARRAY_AREF(arg1, i));
+	for (i = 0; i < num; i++) {
+	    VALUE item = RARRAY_AREF(arg1, i);
+	    bignums[i] = GetBNPtr(item);
+	    rb_ary_push(bns_tmp, item);
+	}
 
 	points = ALLOCV_N(const EC_POINT *, tmp_p, num);
 	points[0] = point_self; /* self */