Hi all,
BN_bin2bn() accepts integer as the length of input data. If this value
is negative, it will be converted to big positive value in local
variable 'n' inside the function as a result of implicit conversion from
'int' to 'unsigned int'.
In such case, BN_bin2bn() only stops executing because of bn_wexpand()
which fails because bn_expand_internal() fails. bn_expand_internal()
fails with BN_R_BIGNUM_TOO_LONG.
Current version of BN_bin2bn looks like this (with comments inserted):
BIGNUM *BN_bin2bn(const unsigned char *s, int len, BIGNUM *ret)
{
unsigned int i,m;
unsigned int n;
BN_ULONG l;
BIGNUM *bn = NULL;
if (ret == NULL)
ret = bn = BN_new();
if (ret == NULL) return(NULL);
bn_check_top(ret);
l=0;
n=len; << conversion happens here
if (n == 0)
{
ret->top=0;
return(ret);
}
i=((n-1)/BN_BYTES)+1;
m=((n-1)%(BN_BYTES));
if (bn_wexpand(ret, (int)i) == NULL) << check fails here for negative n
{
if (bn) BN_free(bn);
return NULL;
}
ret->top=i;
ret->neg=0;
while (n--)
{
l=(l<<8L)| *(s++);
if (m-- == 0)
{
ret->d[--i]=l;
l=0;
m=BN_BYTES-1;
}
}
/* need to call this due to clear byte at top if avoiding
* having the top bit set (-ve number) */
bn_correct_top(ret);
return(ret);
}
I wonder why the 'len' parameter is signed integer. If it is not needed
maybe we can just insert a check like this:
BIGNUM *BN_bin2bn(const unsigned char *s, int len, BIGNUM *ret)
{
unsigned int i,m;
unsigned int n;
BN_ULONG l;
BIGNUM *bn = NULL;
+ if (len < 0) return NULL;
if (ret == NULL)
ret = bn = BN_new();
...
v.
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majord...@openssl.org
