[Date Prev][Date Next] [Chronological] [Thread] [Top]

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) (
>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.


>The simple fix for that is to set (*vals)[nvals+j) = NULL before the check
>for zero length. Diff included below.
>--- value.c	Tue Mar  2 18:30:06 1999
>+++ /a/home/paul/ldap/servers/slapd/value.c	Fri Sep 24 00:58:14 1999
>@@ -35,6 +35,7 @@
> 	}
> 	for ( i = 0, j = 0; i < naddvals; i++, j++ ) {
>+		(*vals)[nvals + j] = NULL;
> 		if ( addvals[i]->bv_len > 0 ) {
> 			(*vals)[nvals + j] = ber_bvdup( addvals[i] );
> 		}