Bug 458675 – Memory leaks in valueset code

Bugzilla will be upgraded to version 5.0. The upgrade date is tentatively scheduled for 2 December 2018, pending final testing and feedback.

Bug 458675 - Memory leaks in valueset code

Summary:	Memory leaks in valueset code

Status:	CLOSED ERRATA

Aliases:	None

Product:	389
Classification:	Retired
Component:	Directory Server (Show other bugs)
Sub Component:
Version:	1.1.1
Hardware:	All Linux

Priority	medium Severity medium
Target Milestone:	---
Target Release:	---
Assigned To:	Rich Megginson
QA Contact:	Chandrasekar Kannan
Docs Contact:

URL:
Whiteboard:	comment#1.review+nhosoi
Keywords:	Security

Depends On:
Blocks:	249650 FDS112 453229 CVE-2008-3283
	Show dependency tree / graph

Reported:	2008-08-11 10:25 EDT by Rich Megginson
Modified:	2015-01-04 18:33 EST (History)
CC List:	6 users (show)

See Also:
Fixed In Version:
Doc Type:	Bug Fix
Doc Text:
Story Points:	---
Clone Of:
Environment:
Last Closed:	2008-08-27 16:39:18 EDT
Type:	---
Regression:	---
Mount Type:	---
Documentation:	---
CRM:
Verified Versions:
Category:	---
oVirt Team:	---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---

Attachments	(Terms of Use)
diffs (2.99 KB, patch) 2008-08-11 10:38 EDT, Rich Megginson	no flags	Details \| Diff
cvs commit log - DS8.0 (181 bytes, text/plain) 2008-08-12 18:29 EDT, Rich Megginson	no flags	Details
cvs commit log - HEAD (1.65 KB, text/plain) 2008-08-27 17:09 EDT, Rich Megginson	no flags	Details
Add an attachment (proposed patch, testcase, etc.)

External Trackers
Tracker	ID	Priority	Status	Summary	Last Updated
Red Hat Product Errata	RHSA-2008:0602	normal	SHIPPED_LIVE	Moderate: redhat-ds-base and redhat-ds-admin security and bug fix update	2008-08-27 16:38:30 EDT

Groups: None (edit)

Description Rich Megginson 2008-08-11 10:25:30 EDT

The first leak occurs when you are using replication and you add values to an attribute that were previously deleted - that is, the values that you want to add are on the attribute's deleted values list and are being "resurrected".  This leak is caused by an improper bit test (foo & bar|baz).  The or | has higher precedence and is evaluated first.  The fix is to use parentheses (foo & (bar|baz)).
The second leak is caused when several values are being added to an attribute, and the list contains non-sequential duplicate values (e.g. foo, bar, baz, foo).  The code uses an array of Slapi_Value* called keyvals.  When a valid value is found, the Slapi_Value* is moved from keyvals to valuetreep and the keyvals array index is set to NULL.  This array is passed to valuearray_free to free the individual Slapi_Value* and the array itself.  This works fine in the non-error case because there are no Slapi_Value* elements to free, so it just frees the array.  However, in the duplicate value case, some of the elements have already been set to NULL, so those are skipped over by valuearray_free.  The fix is to introduce a new function valuearray_free_ext that takes an additional argument which is the array index to start freeing from.  That way the non-NULL Slapi_Value* elements can be freed along with the array itself.

Comment 1 Rich Megginson 2008-08-11 10:38:10 EDT

Created attachment 313970 [details]
diffs

Comment 2 Rich Megginson 2008-08-11 12:47:15 EDT

These leaks can only be triggered by an authenticated user who has the ability to add/modify attributes to an entry.  You also have to be using replication.  This bug can be mitigated by turning off the selfwrite access control and by making sure only administrative users can write.

Comment 3 Rich Megginson 2008-08-12 18:29:52 EDT

Created attachment 314149 [details]
cvs commit log - DS8.0

Reviewed by: nkinder,nhosoi (Thanks!)
Fix Description: This leak is caused when several values are being added to an attribute,
and the list contains non-sequential duplicate values (e.g. foo, bar, baz,
foo).  The code uses an array of Slapi_Value* called keyvals.  When a valid
value is found, the Slapi_Value* is moved from keyvals to valuetreep and the
keyvals array index is set to NULL.  This array is passed to valuearray_free to
free the individual Slapi_Value* and the array itself.  This works fine in the
non-error case because there are no Slapi_Value* elements to free, so it just
frees the array.  However, in the duplicate value case, some of the elements
have already been set to NULL, so those are skipped over by valuearray_free.
The fix is to introduce a new function valuearray_free_ext that takes an
additional argument which is the array index to start freeing from.  That way
the non-NULL Slapi_Value* elements can be freed along with the array itself.
Platforms tested: RHEL5, Fedora 8
Flag Day: no
Doc impact: no
QA impact: should be covered by regular nightly and manual testing
New Tests integrated into TET: none

Comment 5 Jenny Galipeau 2008-08-19 16:30:40 EDT

How can QE verify this?  What to look for in the valgrind output?

Comment 6 Rich Megginson 2008-08-19 16:41:55 EDT

(In reply to comment #5)
> How can QE verify this?  What to look for in the valgrind output?

Look for leaks in valuetree_add_valuearray() or entry_add_present_values_wsi()

Comment 7 Jenny Galipeau 2008-08-21 13:48:18 EDT

verified 8.0 RHEL4-32, RHEL4-64, RHEL5-32, RHEL5-64

Comment 10 errata-xmlrpc 2008-08-27 16:39:18 EDT

An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHSA-2008-0602.html

Comment 11 Rich Megginson 2008-08-27 17:09:54 EDT

Created attachment 315149 [details]
cvs commit log - HEAD

Note You need to log in before you can comment on or make changes to this bug.