Forgot to cc [EMAIL PROTECTED]
On Mon, 04 Feb 2002, Samuel Meder wrote:
>
> The bug occurs when reading objects that are bigger than the SSL
> record size from a SSL bio:
>
> ASN1_d2i_bio calls BIO_read without checking that the amount received
> is equal to the amount requested (This causes a problem becuase
> BIO_read will never return more than a record worth of data when
> reading from a ssl bio). The following patch seems to fix the problem.
>
> To make things clearer here is the current bit of problematic code in
> ASN1_d2i_bio with a few comments:
>
> for (;;)
> {
> if (want >= (len-off))
> {
> want-=(len-off);
>
> if (!BUF_MEM_grow(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) && ((len-off) == 0))
> {
> ASN1err(ASN1_F_ASN1_D2I_BIO,ASN1_R_NOT_ENOUGH_DATA);
> goto err;
> }
> if (i > 0)
> len+=i;
> =============================================================================
> When the header stradles a ssl record boundary, we will have i>0, but
> i<want. This can cause ASN1_get_object to return ASN1_R_TOO_LONG
> (which is seemingly ignored here) and will (I think) lead to accessing
> uninitialized data. Note that my patch doesn't address this one as I'm
> less sure of it.
> =============================================================================
>
> }
> /* else data already loaded */
>
> p=(unsigned char *)&(b->data[off]);
> c.p=p;
> c.inf=ASN1_get_object(&(c.p),&(c.slen),&(c.tag),&(c.xclass),
> len-off);
> if (c.inf & 0x80)
> {
> unsigned long e;
>
> e=ERR_GET_REASON(ERR_peek_error());
> if (e != ASN1_R_TOO_LONG)
> goto err;
> else
> ERR_get_error(); /* clear error */
> }
> i=c.p-p;/* header length */
> off+=i; /* end of data */
>
> if (c.inf & 1)
> {
> /* no data body so go round again */
> eos++;
> want=HEADER_SIZE;
> }
> else if (eos && (c.slen == 0) && (c.tag == V_ASN1_EOC))
> {
> /* eos value, so go back and read another header */
> eos--;
> if (eos <= 0)
> break;
> else
> want=HEADER_SIZE;
> }
> else
> {
> /* suck in c.slen bytes of data */
> want=(int)c.slen;
> if (want > (len-off))
> {
> want-=(len-off);
> if (!BUF_MEM_grow(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)
> {
> ASN1err(ASN1_F_ASN1_D2I_BIO,ASN1_R_NOT_ENOUGH_DATA);
> goto err;
> }
> len+=i;
> =============================================================================
> Here we have a definite problem. Again if we hit the situation where
> i>0, but i<want, we just ignore this and return successfully.
> =============================================================================
> }
> off+=(int)c.slen;
> if (eos <= 0)
> {
> break;
> }
> else
> want=HEADER_SIZE;
> }
> }
>
> I've attached a patch for the second problem, though I believe a
> similar thing should be done for the first.
>
> /Sam
> diff -u -r openssl-0.9.6c/crypto/asn1/a_d2i_fp.c
>openssl-0.9.6c-patched/crypto/asn1/a_d2i_fp.c
> --- openssl-0.9.6c/crypto/asn1/a_d2i_fp.c Fri Apr 23 17:08:07 1999
> +++ openssl-0.9.6c-patched/crypto/asn1/a_d2i_fp.c Sun Feb 3 14:26:45 2002
> @@ -169,13 +169,17 @@
>
>ASN1err(ASN1_F_ASN1_D2I_BIO,ERR_R_MALLOC_FAILURE);
> goto err;
> }
> - i=BIO_read(in,&(b->data[len]),want);
> - if (i <= 0)
> - {
> -
>ASN1err(ASN1_F_ASN1_D2I_BIO,ASN1_R_NOT_ENOUGH_DATA);
> - goto err;
> - }
> - len+=i;
> + while(want)
> + {
> + i=BIO_read(in,&(b->data[len]),want);
> + if (i <= 0)
> + {
> +
>ASN1err(ASN1_F_ASN1_D2I_BIO,ASN1_R_NOT_ENOUGH_DATA);
> + goto err;
> + }
> + len+=i;
> + want-=i;
> + }
> }
> off+=(int)c.slen;
> if (eos <= 0)
>
>
>
>
>
>
>
>
>
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
