[xmlsec] Re: A bug in the xmlSecBnFromString function

[Aleksey Sanin]

Hi, Michael!

Thanks for the patch. Applied and commited!

Aleksey

Michael Mi wrote:
> Hi, Aleksey,
> 
> I found a bug in the function "xmlSecBnFromString" in file 
> libxmlsec/src/bn.c.
> 
> Suppose that there is a string "3613992633088206991095317234205295", 
> which is a positive number.
> The xmlSecBnFromString function converts this string into "B2 2F 00 00 
> 00 02 20 73 3B 25 34 C4 42 6F", a negative number.
> 
> Because of this difference, when I try to find a certificate with above 
> serial number, the cert can't be found. (mscrypto api)
> 
> I patched this function, that is, when a bn representing a positive 
> number, has its first byte bigger than 127, a "00" prefex is added.
> 
> Please confirm.
> 
> Thanks
> 
> Michael
> 
> 
> =========================================================================
> int
> xmlSecBnFromString(xmlSecBnPtr bn, const xmlChar* str, xmlSecSize base) {
>     xmlSecSize i, len;
>     xmlSecByte ch;
>     int nn;
>     int ret;
> 
>    * /*
>      * mmi : added for adding prefix 00
>      */
>     xmlSecByte* data;
> *
>     xmlSecAssert2(bn != NULL, -1);
>     xmlSecAssert2(str != NULL, -1);
>     xmlSecAssert2(base > 1, -1);
>     xmlSecAssert2(base <= sizeof(xmlSecBnRevLookupTable), -1);
> 
>     /* trivial case */
>     len = xmlStrlen(str);
>     if(len == 0) {
>     return(0);
>     }
>    
>    * /* The result size could not exceed the input string length
>      * because each char fits inside a byte in all cases :)
>      * In truth, it would be likely less than 1/2 input string length
>      * because each byte is represented by 2 chars. If needed,
>      * buffer size would be increased by Mul/Add functions.
>      */
>     //ret = xmlSecBufferSetMaxSize(bn, xmlSecBufferGetSize(bn) + len / 2 
> + 1);
> 
>     /*
>      * mmi:add one byte to hold the 00 prefix
>      */
>     ret = xmlSecBufferSetMaxSize(bn, xmlSecBufferGetSize(bn) + len / 2 + 
> 2);*
> 
>     if(ret < 0) {
>     xmlSecError(XMLSEC_ERRORS_HERE,
>             NULL,
>             "xmlSecBnRevLookupTable",
>             XMLSEC_ERRORS_R_XMLSEC_FAILED,
>             "size=%d", len / 2 + 1);
>     return (-1);
>     }
> 
>     for(i = 0; i < len; i++) {
>     ch = str[i];
>     if(isspace(ch)) {
>         continue;
>     }
> 
>     xmlSecAssert2(ch <= sizeof(xmlSecBnLookupTable), -1);
>     nn = xmlSecBnLookupTable[ch];
>     if((nn < 0) || ((xmlSecSize)nn > base)) {
>         xmlSecError(XMLSEC_ERRORS_HERE,
>             NULL,
>             NULL,
>             XMLSEC_ERRORS_R_INVALID_DATA,
>             "char=%c;base=%d",
>             ch, base);
>             return (-1);
>     }
>    
>     ret = xmlSecBnMul(bn, base);
>     if(ret < 0) {
>         xmlSecError(XMLSEC_ERRORS_HERE,
>             NULL,
>             "xmlSecBnMul",
>             XMLSEC_ERRORS_R_XMLSEC_FAILED,
>             "base=%d", base);
>         return (-1);
>     }
> 
>     ret = xmlSecBnAdd(bn, nn);
>     if(ret < 0) {
>         xmlSecError(XMLSEC_ERRORS_HERE,
>             NULL,
>             "xmlSecBnAdd",
>             XMLSEC_ERRORS_R_XMLSEC_FAILED,
>             "base=%d", base);
>         return (-1);
>     }   
> 
>     }
> 
>    * /*
>      * mmi : check whether need to add 00 prefix
>      */
>     data = xmlSecBufferGetData(bn);
>     if (data[0]>127)
>     {
>         ch = 0;
>         ret = xmlSecBufferPrepend(bn, &ch, 1);
>         if(ret < 0) {
>             xmlSecError(XMLSEC_ERRORS_HERE,
>                 NULL,
>                 "xmlSecBufferPrepend",
>                 XMLSEC_ERRORS_R_XMLSEC_FAILED,
>                 "base=%d", base);
>             return (-1);
>         }
>     }*
> 
>     return(0);
> }
> 
> 
>