This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 458510 - Memory leak setting password with passwd extop
Memory leak setting password with passwd extop
Status: CLOSED ERRATA
Product: 389
Classification: Community
Component: Security - Password Policy (Show other bugs)
1.1.1
All Linux
medium Severity medium
: ---
: ---
Assigned To: Rich Megginson
Chandrasekar Kannan
: Security
Depends On:
Blocks: 249650 FDS112 453229 CVE-2008-3283
  Show dependency treegraph
 
Reported: 2008-08-08 22:24 EDT by Rich Megginson
Modified: 2015-01-04 18:33 EST (History)
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:11 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
diffs (2.40 KB, patch)
2008-08-08 22:25 EDT, Rich Megginson
no flags Details | Diff
cvs commit log - DS8.0 (193 bytes, text/plain)
2008-08-12 18:27 EDT, Rich Megginson
no flags Details
cvs commit log - HEAD (187 bytes, text/plain)
2008-08-27 17:08 EDT, Rich Megginson
no flags Details

  None (edit)
Description Rich Megginson 2008-08-08 22:24:15 EDT
1) if the given dn is "", that 1 byte will be leaked when the dn is reassigned to the bind dn
2) calling slapi_pblock_get with SLAPI_CONN_DN does a strdup, which is different than most uses of slapi_pblock_get.  That memory must be freed.
3) If we set the ORIGINAL_TARGET to a dn other than the given dn, we must free it
Comment 1 Rich Megginson 2008-08-08 22:25:27 EDT
Created attachment 313861 [details]
diffs
Comment 2 Noriko Hosoi 2008-08-09 12:44:02 EDT
(In reply to comment #0)

> 3) If we set the ORIGINAL_TARGET to a dn other than the given dn, we must free
> it

It looks to me that when a dn other than the given dn is set to ORIGINAL_TARGET, the given dn (dn == "") is already freed by your fix.  And the dn set to ORIGINAL_TARGET is the one duplicated from "bindDN", which is freed at the line 753.  But I should be missing something.  Are there any place "dn" is overwritten after setting to ORIGINAL_TARGET?
    660      /* Determine the target DN for this operation */
    661      /* Did they give us a DN ? */
    662      if (dn == NULL || *dn == '\0') {
    663         /* Get the DN from the bind identity on this connection */
                slapi_ch_free_string(&dn);
    664         dn = slapi_ch_strdup(bindDN);
    665         LDAPDebug( LDAP_DEBUG_ANY,
    666                 "Missing userIdentity in request, using the bind DN inst ead.\n",
    667              0, 0, 0 );
    668      }
    669      
    670      slapi_pblock_set( pb, SLAPI_ORIGINAL_TARGET, dn );
    671 

    749     /* Either this is the same pointer that we allocated and set above,
    750      * or whoever used it should have freed it and allocated a new
    751      * value that we need to free here */
    752     slapi_pblock_get( pb, SLAPI_ORIGINAL_TARGET, &dn );
    753     slapi_ch_free_string(&dn);
    754     slapi_pblock_set( pb, SLAPI_ORIGINAL_TARGET, NULL );
Comment 3 Rich Megginson 2008-08-11 10:46:19 EDT
(In reply to comment #2)
> (In reply to comment #0)
> 
> > 3) If we set the ORIGINAL_TARGET to a dn other than the given dn, we must free
> > it
> 
> It looks to me that when a dn other than the given dn is set to
> ORIGINAL_TARGET, the given dn (dn == "") is already freed by your fix.

Not necessarily.  There are several error conditions earlier in the function that can cause the code to jump to free_and_return: after dn has been allocated.  If this happens, we must free dn.

> And the
> dn set to ORIGINAL_TARGET is the one duplicated from "bindDN", which is freed
> at the line 753.  But I should be missing something.  Are there any place "dn"
> is overwritten after setting to ORIGINAL_TARGET?
>     660      /* Determine the target DN for this operation */
>     661      /* Did they give us a DN ? */
>     662      if (dn == NULL || *dn == '\0') {
>     663         /* Get the DN from the bind identity on this connection */
>                 slapi_ch_free_string(&dn);
>     664         dn = slapi_ch_strdup(bindDN);
>     665         LDAPDebug( LDAP_DEBUG_ANY,
>     666                 "Missing userIdentity in request, using the bind DN
> inst ead.\n",
>     667              0, 0, 0 );
>     668      }
>     669      
>     670      slapi_pblock_set( pb, SLAPI_ORIGINAL_TARGET, dn );
>     671 
> 
>     749     /* Either this is the same pointer that we allocated and set above,
>     750      * or whoever used it should have freed it and allocated a new
>     751      * value that we need to free here */
>     752     slapi_pblock_get( pb, SLAPI_ORIGINAL_TARGET, &dn );
>     753     slapi_ch_free_string(&dn);
>     754     slapi_pblock_set( pb, SLAPI_ORIGINAL_TARGET, NULL );
Comment 4 Nathan Kinder 2008-08-11 11:27:59 EDT
The fix looks fine, but I am also curious about Noriko's second question.  In what cases is SLAPI_ORIGINAL_TARGET changed to something other than the "dn" pointer?
Comment 5 Rich Megginson 2008-08-11 12:38:11 EDT
This leak can be triggered by an anonymous user.  There is no way I know of to mitigate this issue.
Comment 6 Rich Megginson 2008-08-12 18:27:50 EDT
Created attachment 314146 [details]
cvs commit log - DS8.0

Reviewed by: nkinder (Thanks!)
Fix Description: 1) if the given dn is "", that 1 byte will be leaked when the dn is reassigned
to the bind dn - so free it first in that case before reassigning
2) calling slapi_pblock_get with SLAPI_CONN_DN does a strdup, which is
different than most uses of slapi_pblock_get.  That memory must be freed.  So we free it at the end.
3) If we set the ORIGINAL_TARGET to a dn other than the given dn, we must free
it - grab it and compare it to dn - if not the same, free dn first, then free the original target dn
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 8 Jenny Galipeau 2008-08-19 16:28:50 EDT
How can QE verify this?  What to look for in the valgrind output?
Comment 9 Rich Megginson 2008-08-19 16:35:34 EDT
(In reply to comment #8)
> How can QE verify this?  What to look for in the valgrind output?

Look for a memory leak in passwd_modify_extop()
Comment 10 Jenny Galipeau 2008-08-21 13:46:58 EDT
verified 8.0 RHEL4-32, RHEL4-64, RHEL5-32, RHEL5-64
Comment 13 errata-xmlrpc 2008-08-27 16:39:11 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 14 Rich Megginson 2008-08-27 17:08:27 EDT
Created attachment 315146 [details]
cvs commit log - HEAD

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