null attributes can crash the server

[paul@originative.co.uk]

I've tracked down a problem whereby slapd would core dump with a particular
set of data that a client was using.

The problem was that the LDIF data had a description attribute which
consisted *solely* of a continuation line, i.e. a newline and a space

My first question is whether this would be considered a valid attribute
value or not? I can't find anything that suggests it wouldn't be, I'd expect
it to be equivalent to a null attribute value.

There are two potential bugs involved.

The first has to do with ldif conversion. The ldif library function
str_getline() in line64.c converts a continuation line into a single
non-space binary character and will return a valid string containing that
one character. When this string is passed to str_parse_line() the code that
removes continuation lines is after the code that checks whether the value
is null. This means the str_parse_line(), instead of returning an error
returns a valid attribute of length 0.

The simple fix for that is to do the removal of continuation characters
before the check for a null attribute value.

The second bug has to do with adding attribute values. 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.

The simple fix for that is to set (*vals)[nvals+j) = NULL before the check
for zero length.

If the ldif bug is fixed then it's not likely that the value_add_fast() bug
would ever get triggered but the additional overhead of the fix is probably
worth it to prevent server crashes should other bugs result in null
attribute values being added.

Paul Richards
Originative Solutions Ltd