300482 - ldap_multisort_entries incorrectly return an error code on zero length results

Status: RESOLVED FIXED

(Directory :: LDAP C SDK, defect)

Description

[Olivier Jolly]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.8) Gecko/20050517 Firefox/1.0.4 (Debian package 1.0.4-2)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.8) Gecko/20050517 Firefox/1.0.4 (Debian package 1.0.4-2)

if ldap_multisort_entries is called to sort a search result with 0 entry, it
returns a memory error when doing et = (struct entrything *)NSLDAPI_MALLOC(
count * sizeof(struct entrything) )) since count = 0.
It should behave normally and return a success error code, even if doing nothing.

Reproducible: Always

Steps to Reproduce:
1.Compile a binary using the c directory, with a sort on client side
2.Run the binary and go via this sort while the filter returns no entry


Actual Results:  
Error code 90 (memory error) is raised

Expected Results:  
No error code with an empty result

The issue was generated under sparc solaris 8

Comment 1

[Olivier Jolly]

Attachment: Proposition of patch

I worked around the problem with this kind of patch

Comment 2

[Mark Smith [:mcs]]

Thnaks for reporting this bug and providing a patch.  I might change the check
for count == 0 to count < 2 (no point in sorting one entry either).  I will
commit the fix soon.

Comment 3

[Olivier Jolly]

I had not used < 2 on purposes as count can be == -1 if ldap_count_entries fails
to count the entries.
On another side, if the count is -1, the malloc of a negative value could be
devastating, but the ldap_multisort_entries should then return an error rather
than a success as if there were only 0 or 1 entries.

Comment 4

[Rich Megginson]

Mark, can you reassign this to me unless you already have a fix?  Thanks.

Comment 5

[Rich Megginson]

Attachment: diffs for fix

I noticed the same problem in keysort above.  This version first checks for count < 0 which is the error case.  Then it just returns 0 in case count < 2.

Comment 6

[Mark Smith [:mcs]]

Comment on attachment 231591 [details]
diffs for fix

OK.

Comment 7

[Rich Megginson]

Checking in sort.c;
/cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap/sort.c,v  <--  sort.c
new revision: 5.3; previous revision: 5.2
done