RE: Adding zero length attribute values can crash slapd (ITS#303)

[paul@originative.co.uk]

> -----Original Message-----
> From: Kurt@OpenLDAP.org [mailto:Kurt@OpenLDAP.org]
> Sent: 24 September 1999 02:10
> To: openldap-its@OpenLDAP.org
> Subject: Re: Adding zero length attribute values can crash slapd
> (ITS#303)
> 
> 
> At 12:00 AM 9/24/99 GMT, paul@originative.co.uk wrote:
> >Full_Name: Paul Richards
> >Version: 1.2.7
> >OS: FreeBSD
> >URL: ftp://ftp.openldap.org/incoming/
> >Submission from: (NULL) (194.217.50.234)
> >
> >
> >In the function value_add_fast() in servers/slapd/value.c, 
> in the for loop that
> >creates the new attribute values there is a check to see if 
> the value has a
> >length > 0.
> >
> >The bug occurs if this for loop is executed when only a 
> single, zero length
> >value is to be added, in which case the loop terminates 
> having not done
> >anything and with j=1. The following code that puts NULL 
> into the last
> >element of the array therefore puts it in (*vals)[1], 
> however, nothing at
> >all has been written to (*vals)[0] and the data in 
> (*vals)[0] is therefore
> >indeterminate. Under the right circumstances that value will 
> not be NULL and
> >will cause slapd to crash when that attribute is later accessed.
> 
> I don't believe this code is correct.  If the first value is empty
> and the second contains data, you end up { NULL, {DATA}, NULL }
> in the vector.
> 
> I'm sure that slapd likely has many places where it cannot handle
> an empty value, hence for now, we should ignore the value.
> 
> I think the quick fix would be to increment j only if a value
> was set.

yeah, you're right. The correct fix would be


for ( i = 0, j = 0; i < naddvals; i++) {
	if ( addvals[i]->bv_len > 0 ) {
		(*vals)[nvals + j] = ber_bvdup( addvals[i] );
		j++;
	}


Paul Richards
Originative Solutions Ltd