Hello,
my colleague Jan Hofmann experimented with new ASN.1 code from
openssl-0.9.7-beta4. He achieved buggy behavior when parsing
incomplete (truncated) DER data under specific conditions.
Openssl does'not detect any error while parsing truncated DER data.
He asked on openssl-users mailling list without response.
I tried to debug his code down into libcrypto and localized bug
in function asn1_d2i_read_bio().
I found this problem in RT/openssl.org since May 2002 (Id #20).
NOTE: Rsync protocol on dev.openssl.org is down now, so I can't
view the latest crypto/asn1/a_d2i_fp.c (update local CVS mirror).
There is the last and only mail in RT:
> [[EMAIL PROTECTED] - Sat May 4 20:44:23 2002]:
>
> > Experimenting with "openssl smime -decrypt", I found that it did not
> > detect that a
> > message was truncated. Changing line 173 of crypto/asn1/a_d2i_fp.c
> > from
> > if (i <= 0)
> > to
> > if (i < want)
> > fixes the problem. I think this is the right code for all cases, but
> > somebody who
> > actually understands the whole ASN parsing framework should probably
> > check it out.
> >
>
> Its a bit more complex than that. The reason for the <=0 test is because
> the actual value for 'want' is not always accurate. In particular when a
> header is being read 'want' is set to HEADER_SIZE which is 8.
>
> This is a only upper bound for a sensible header size. A valid header
> may only be two octets in length: 0x30, 0x0 for example is a zero
> length SEQUENCE.
>
> So what is actually needed is two different techniques, one to read
> in the header and the other the content octets (assuming their
> size is accurately known).
>
> Steve.
My try to solve problem is attached. I added variable 'require'
parallel to 'want'...
I have written this patch only on facts mentioned in Steve's
mail. I have not analysed code deeply. Review is needed.
I would be pleased, if you would be so kind and have look at this
serious bug. Thank you very much.
--
Zito
--- openssl-0.9.7-beta4/crypto/asn1/a_d2i_fp.c.Orig Tue Dec 3 10:26:18 2002
+++ openssl-0.9.7-beta4/crypto/asn1/a_d2i_fp.c Tue Dec 3 12:36:46 2002
@@ -140,6 +140,7 @@
#endif
#define HEADER_SIZE 8
+#define HEADER_SIZE_MIN 2
static int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)
{
BUF_MEM *b;
@@ -148,6 +149,7 @@
int ret=-1;
ASN1_CTX c;
int want=HEADER_SIZE;
+ int require=HEADER_SIZE_MIN;
int eos=0;
int off=0;
int len=0;
@@ -165,6 +167,7 @@
if (want >= (len-off))
{
want-=(len-off);
+ require-=(len-off);
if (!BUF_MEM_grow_clean(b,len+want))
{
@@ -172,7 +175,7 @@
goto err;
}
i=BIO_read(in,&(b->data[len]),want);
- if ((i < 0) && ((len-off) == 0))
+ if (i < require)
{
ASN1err(ASN1_F_ASN1_D2I_BIO,ASN1_R_NOT_ENOUGH_DATA);
goto err;
@@ -204,6 +207,7 @@
/* no data body so go round again */
eos++;
want=HEADER_SIZE;
+ require=HEADER_SIZE_MIN;
}
else if (eos && (c.slen == 0) && (c.tag == V_ASN1_EOC))
{
@@ -213,21 +217,24 @@
break;
else
want=HEADER_SIZE;
+ require=HEADER_SIZE_MIN;
}
else
{
/* suck in c.slen bytes of data */
want=(int)c.slen;
+ require=want;
if (want > (len-off))
{
want-=(len-off);
+ require=want;
if (!BUF_MEM_grow_clean(b,len+want))
{
ASN1err(ASN1_F_ASN1_D2I_BIO,ERR_R_MALLOC_FAILURE);
goto err;
}
i=BIO_read(in,&(b->data[len]),want);
- if (i <= 0)
+ if (i < require)
{
ASN1err(ASN1_F_ASN1_D2I_BIO,ASN1_R_NOT_ENOUGH_DATA);
goto err;
@@ -241,6 +248,7 @@
}
else
want=HEADER_SIZE;
+ require=HEADER_SIZE_MIN;
}
}
