Probably bug

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Probably bug

Tarasov Viktor
Hi,

there is the bug in trunk version,
when decoding SC_ASN1_UTF8STRING entry with SC_ASN1_ALLOC flag.

Allocated output buffer size is in_len-1 (src/libopensc/asn1.c +981),
after that, inside the sc_asn1_decode_utf8string(),
there is the test that fails when in_len+1 is greater then out_len
(src/libopensc/asn1.c +649).

Regards,
Viktor.

--- ./opensc.trunk/src/libopensc/asn1.c.orig    2005-08-26
14:19:55.000000000 +0200
+++ ./opensc.trunk/src/libopensc/asn1.c 2005-08-26 14:20:26.000000000 +0200
@@ -978,12 +978,12 @@
                        assert(len != NULL);
                        if (entry->flags & SC_ASN1_ALLOC) {
                                u8 **buf = (u8 **) parm;
-                               *buf = (u8 *) malloc(objlen-1);
+                               *buf = (u8 *) malloc(objlen+1);
                                if (*buf == NULL) {
                                        r = SC_ERROR_OUT_OF_MEMORY;
                                        break;
                                }
-                               *len = objlen-1;
+                               *len = objlen+1;
                                parm = *buf;
                        }
                        r = sc_asn1_decode_utf8string(obj, objlen, (u8
*) parm, len);









_______________________________________________
opensc-devel mailing list
[hidden email]
http://www.opensc.org/cgi-bin/mailman/listinfo/opensc-devel
Reply | Threaded
Open this post in threaded view
|

Re: Probably bug

Nils Larsch
Hi Viktor,

Tarasov Viktor wrote:

> Hi,
>
> there is the bug in trunk version,
> when decoding SC_ASN1_UTF8STRING entry with SC_ASN1_ALLOC flag.
>
> Allocated output buffer size is in_len-1 (src/libopensc/asn1.c +981),
> after that, inside the sc_asn1_decode_utf8string(),
> there is the test that fails when in_len+1 is greater then out_len
> (src/libopensc/asn1.c +649).
>
> Regards,
> Viktor.
>
> --- ./opensc.trunk/src/libopensc/asn1.c.orig    2005-08-26
> 14:19:55.000000000 +0200
> +++ ./opensc.trunk/src/libopensc/asn1.c 2005-08-26 14:20:26.000000000 +0200
> @@ -978,12 +978,12 @@
>                         assert(len != NULL);
>                         if (entry->flags & SC_ASN1_ALLOC) {
>                                 u8 **buf = (u8 **) parm;
> -                               *buf = (u8 *) malloc(objlen-1);
> +                               *buf = (u8 *) malloc(objlen+1);
>                                 if (*buf == NULL) {
>                                         r = SC_ERROR_OUT_OF_MEMORY;
>                                         break;
>                                 }
> -                               *len = objlen-1;
> +                               *len = objlen+1;
>                                 parm = *buf;
>                         }
>                         r = sc_asn1_decode_utf8string(obj, objlen, (u8
> *) parm, len);

you're right, decreasing the object length (== length of the UTF-8
string) doesn't make much sense and to be honest I wonder why no
one has noticed this before. However I'm not sure whether it's really
a good decision to increase the size by one (for the '\0' character)
as a asn.1 decoding function should return what has been encoded and
not append a zero character (but I'm not sure if this will cause
problems elsewhere in the opensc code).

Nils
_______________________________________________
opensc-devel mailing list
[hidden email]
http://www.opensc.org/cgi-bin/mailman/listinfo/opensc-devel
Reply | Threaded
Open this post in threaded view
|

Re: Probably bug

Tarasov Viktor
Nils Larsch wrote:

> Hi Viktor,
>
> Tarasov Viktor wrote:
>
>> Hi,
>>
>> there is the bug in trunk version,
>> when decoding SC_ASN1_UTF8STRING entry with SC_ASN1_ALLOC flag.
>>
>> Allocated output buffer size is in_len-1 (src/libopensc/asn1.c +981),
>> after that, inside the sc_asn1_decode_utf8string(),
>> there is the test that fails when in_len+1 is greater then out_len
>> (src/libopensc/asn1.c +649).
>>
>> Regards,
>> Viktor.
>>
>> --- ./opensc.trunk/src/libopensc/asn1.c.orig    2005-08-26
>> 14:19:55.000000000 +0200
>> +++ ./opensc.trunk/src/libopensc/asn1.c 2005-08-26 14:20:26.000000000
>> +0200
>> @@ -978,12 +978,12 @@
>>                         assert(len != NULL);
>>                         if (entry->flags & SC_ASN1_ALLOC) {
>>                                 u8 **buf = (u8 **) parm;
>> -                               *buf = (u8 *) malloc(objlen-1);
>> +                               *buf = (u8 *) malloc(objlen+1);
>>                                 if (*buf == NULL) {
>>                                         r = SC_ERROR_OUT_OF_MEMORY;
>>                                         break;
>>                                 }
>> -                               *len = objlen-1;
>> +                               *len = objlen+1;
>>                                 parm = *buf;
>>                         }
>>                         r = sc_asn1_decode_utf8string(obj, objlen, (u8
>> *) parm, len);
>
>
> you're right, decreasing the object length (== length of the UTF-8
> string) doesn't make much sense and to be honest I wonder why no
> one has noticed this before.

I guess that nobody used SC_ASN1_ALLOC flag for SC_ASN1_UTF8STRING.
(src/libopensc/asn1.c +903)
(By the way, the similar allocation I see for the SC_ASN1_BIT_STRING
entry as well, is it correct?
I have not used it yet.)

> However I'm not sure whether it's really
> a good decision to increase the size by one (for the '\0' character)
> as a asn.1 decoding function should return what has been encoded and
> not append a zero character (but I'm not sure if this will cause
> problems elsewhere in the opensc code).


> Nils
>
>

_______________________________________________
opensc-devel mailing list
[hidden email]
http://www.opensc.org/cgi-bin/mailman/listinfo/opensc-devel
Reply | Threaded
Open this post in threaded view
|

Re: Probably bug

Nils Larsch
Tarasov Viktor wrote:
...
> (By the way, the similar allocation I see for the SC_ASN1_BIT_STRING
> entry as well, is it correct?
> I have not used it yet.)

yes, and I guess the code for the UTF-8 strings was simply
copy&pasted from the bit string code. In case of bit strings
one octet in the encoding is used to specify the number of
unused bits in the topmost byte and this octet is dropped
when decoding.

Nils
_______________________________________________
opensc-devel mailing list
[hidden email]
http://www.opensc.org/cgi-bin/mailman/listinfo/opensc-devel