Bug 458510
Summary: | Memory leak setting password with passwd extop | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Retired] 389 | Reporter: | Rich Megginson <rmeggins> | ||||||||
Component: | Security - Password Policy | Assignee: | Rich Megginson <rmeggins> | ||||||||
Status: | CLOSED ERRATA | QA Contact: | Chandrasekar Kannan <ckannan> | ||||||||
Severity: | medium | Docs Contact: | |||||||||
Priority: | medium | ||||||||||
Version: | 1.1.1 | CC: | benl, jgalipea, jlieskov, nhosoi, nkinder, security-response-team | ||||||||
Target Milestone: | --- | Keywords: | Security | ||||||||
Target Release: | --- | ||||||||||
Hardware: | All | ||||||||||
OS: | Linux | ||||||||||
Whiteboard: | |||||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||||
Doc Text: | Story Points: | --- | |||||||||
Clone Of: | Environment: | ||||||||||
Last Closed: | 2008-08-27 20:39:11 UTC | Type: | --- | ||||||||
Regression: | --- | Mount Type: | --- | ||||||||
Documentation: | --- | CRM: | |||||||||
Verified Versions: | Category: | --- | |||||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||||
Embargoed: | |||||||||||
Bug Depends On: | |||||||||||
Bug Blocks: | 249650, 452721, 453229, 458977 | ||||||||||
Attachments: |
|
Description
Rich Megginson
2008-08-09 02:24:15 UTC
Created attachment 313861 [details]
diffs
(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 ); (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 ); 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? This leak can be triggered by an anonymous user. There is no way I know of to mitigate this issue. 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
How can QE verify this? What to look for in the valgrind output? (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() verified 8.0 RHEL4-32, RHEL4-64, RHEL5-32, RHEL5-64 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 Created attachment 315146 [details]
cvs commit log - HEAD
|