Skip to content

Commit

Permalink
bn: keep reference to temporary OpenSSL::BN object created by GetBNPtr()
Browse files Browse the repository at this point in the history
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: ruby#87
  • Loading branch information
rhenium committed Dec 4, 2016
1 parent 72126d6 commit 182604c
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 19 deletions.
36 changes: 21 additions & 15 deletions ext/openssl/ossl_bn.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion ext/openssl/ossl_bn.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);


Expand Down
10 changes: 7 additions & 3 deletions ext/openssl/ossl_pkey_ec.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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 */
Expand Down

0 comments on commit 182604c

Please sign in to comment.