[Fedora-directory-commits] ldapserver/ldap/servers/slapd/back-ldbm dn2entry.c, 1.4, 1.5
Richard Allen Megginson (rmeggins)
fedora-directory-commits at redhat.com
Thu Feb 23 20:48:12 UTC 2006
Author: rmeggins
Update of /cvs/dirsec/ldapserver/ldap/servers/slapd/back-ldbm
In directory cvs-int.fedora.redhat.com:/tmp/cvs-serv4939/ldapserver/ldap/servers/slapd/back-ldbm
Modified Files:
dn2entry.c
Log Message:
Bug(s) fixed: 179137
Bug Description: recursion causes OOM with bad DN in dn2ancestor
Reviewed by: All (Thanks!)
Fix Description:
The fix looks scary, but I thought it would be best to get rid of
recursion entirely (ugh - recursion in a multi threaded server - this
isn't lisp . . .). Along with eliminating recursion, I created a new
function called slapi_dn_find_parent that just returns a pointer to the
beginning of the parent of the given dn, rather than returning a copy
(as in slapi_dn_parent), to eliminate malloc/free in cases where it is
unnecessary such as iterating through the parents in an DN. The new
function is basically just the guts of slapi_dn_parent with one twist,
specifically to address the bug in question - it skips through
consecutive runs of DN separator characters. We should probably have a
function like const char *slapi_dn_is_valid(const char *) that returns
NULL if the given DN is valid or returns a pointer to the first invalid
character if not. We could probably save a lot of time in processing
bad or malicious client requests.
Anyway, back to dn2ancestor. The given ancestordn must contain the
_unnormalized_ parent DN, since some clients get irritated when they get
back an DN in a different form than given. However, we need to have a
normalized DN to pass to dn2entry, and we cannot use a single Slapi_DN
that has both a dn and a ndn that are passed in byval (unless we add a
new API or skip the API altogether), so the variable ancestorndn holds
the normalized DN. Using the original pointer to the given sdn also
allows us to avoid malloc/free entirely.
Platforms tested: Fedora Core 4
Flag Day: no
Doc impact: no
QA impact: should be covered by regular nightly and manual testing
New Tests integrated into TET: We need a test case that calls moddn and
modify operations with really bad DNs, consisting of nothing but
thousands of ',', '+', and '=' chars.
Index: dn2entry.c
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/servers/slapd/back-ldbm/dn2entry.c,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -r1.4 -r1.5
--- dn2entry.c 19 Apr 2005 22:07:38 -0000 1.4
+++ dn2entry.c 23 Feb 2006 20:48:05 -0000 1.5
@@ -109,58 +109,12 @@
}
/*
- * dn2entry_or_ancestor - look up dn in the cache/indexes and return the
- * corresponding entry. If the entry is not found, this function returns NULL
- * and sets ancestordn to the DN of highest entry in the tree matched.
- *
- * ancestordn should be initialized before calling this function.
- *
- * When the caller is finished with the entry returned, it should return it
- * to the cache:
- * e = dn2entry_or_ancestor( ... );
- * if ( NULL != e ) {
- * cache_return( &inst->inst_cache, &e );
- * }
- */
-struct backentry *
-dn2entry_or_ancestor(
- Slapi_Backend *be,
- const Slapi_DN *sdn,
- Slapi_DN *ancestordn,
- back_txn *txn,
- int *err
-)
-{
- struct backentry *e;
-
- LDAPDebug( LDAP_DEBUG_TRACE, "=> dn2entry_or_ancestor \"%s\"\n", slapi_sdn_get_dn(sdn), 0, 0 );
-
- /*
- * Fetch the entry asked for.
- */
-
- e= dn2entry(be,sdn,txn,err);
-
- if(e==NULL)
- {
- /*
- * could not find the entry named. crawl back up the dn and
- * stop at the first ancestor that does exist, or when we get
- * to the suffix.
- */
- e= dn2ancestor(be,sdn,ancestordn,txn,err);
- }
-
- LDAPDebug( LDAP_DEBUG_TRACE, "<= dn2entry_or_ancestor %p\n", e, 0, 0 );
- return( e );
-}
-
-/*
* Use the DN to fetch the parent of the entry.
* If the parent entry doesn't exist, keep working
* up the DN until we hit "" or an backend suffix.
*
- * ancestordn should be initialized before calling this function.
+ * ancestordn should be initialized before calling this function, and
+ * should be empty
*
* Returns NULL for no entry found.
*
@@ -184,18 +138,64 @@
LDAPDebug( LDAP_DEBUG_TRACE, "=> dn2ancestor \"%s\"\n", slapi_sdn_get_dn(sdn), 0, 0 );
- /* stop when we get to "", or a backend suffix point */
- slapi_sdn_done(ancestordn); /* free any previous contents */
- slapi_sdn_get_backend_parent(sdn,ancestordn,be);
- if ( !slapi_sdn_isempty(ancestordn) )
- {
- Slapi_DN *newsdn = slapi_sdn_dup(ancestordn);
- e = dn2entry_or_ancestor( be, newsdn, ancestordn, txn, err );
- slapi_sdn_free(&newsdn);
- }
-
- LDAPDebug( LDAP_DEBUG_TRACE, "<= dn2ancestor %p\n", e, 0, 0 );
- return( e );
+ /* first, check to see if the given sdn is empty or a root suffix of the
+ given backend - if so, it has no parent */
+ if (!slapi_sdn_isempty(sdn) && !slapi_be_issuffix( be, sdn )) {
+ Slapi_DN ancestorndn;
+ const char *ptr;
+
+ /* assign ancestordn to the parent of the given dn - ancestordn will contain
+ the "raw" unnormalized DN from the caller, so we can give back the DN
+ in the same format as we received it */
+ ptr = slapi_dn_find_parent(slapi_sdn_get_dn(sdn));
+ /* assign the ancestordn dn pointer to the parent of dn from sdn - sdn "owns"
+ the memory, but ancestordn points to it */
+ slapi_sdn_set_dn_byref(ancestordn, ptr); /* free any previous contents */
+ /* now, do the same for the normalized version */
+ /* ancestorndn holds the normalized version for iteration purposes and
+ because dn2entry needs the normalized dn */
+ ptr = slapi_dn_find_parent(slapi_sdn_get_ndn(sdn));
+ slapi_sdn_init_ndn_byref(&ancestorndn, ptr);
+
+ /*
+ At this point you may be wondering why I need both ancestorndn and
+ ancestordn. Because, with the slapi_sdn interface, you cannot set both
+ the dn and ndn byref at the same time. Whenever you call set_dn or set_ndn,
+ it calls slapi_sdn_done which wipes out the previous contents. I suppose I
+ could have added another API to allow you to pass them both in. Also, using
+ slapi_sdn_get_ndn(ancestordn) every time would result in making a copy then
+ normalizing the copy every time - not efficient.
+ So, why not just use a char* for the ancestorndn? Because dn2entry requires
+ a Slapi_DN with the normalized dn.
+ */
+
+ /* stop when we get to "", or a backend suffix point */
+ while (!e && !slapi_sdn_isempty(&ancestorndn) && !slapi_be_issuffix( be, &ancestorndn )) {
+ /* find the entry - it uses the ndn, so no further conversion is necessary */
+ e= dn2entry(be,&ancestorndn,txn,err);
+ if (!e) {
+ /* not found, so set ancestordn to its parent and try again */
+ ptr = slapi_dn_find_parent(slapi_sdn_get_ndn(&ancestorndn));
+ /* keep in mind that ptr points to the raw ndn pointer inside
+ ancestorndn which is still the ndn string "owned" by sdn, the
+ original dn we started with - we are careful not to touch
+ or change it */
+ slapi_sdn_set_ndn_byref(&ancestorndn, ptr); /* wipe out the previous contents */
+ /* now do the same for the unnormalized one */
+ ptr = slapi_dn_find_parent(slapi_sdn_get_dn(ancestordn));
+ slapi_sdn_set_dn_byref(ancestordn, ptr); /* wipe out the previous contents */
+ }
+ }
+
+ slapi_sdn_done(&ancestorndn);
+ }
+
+ /* post conditions:
+ e is the entry of the ancestor of sdn OR e is the suffix entry
+ OR e is NULL
+ ancestordn contains the unnormalized DN of e or is empty */
+ LDAPDebug( LDAP_DEBUG_TRACE, "<= dn2ancestor %p\n", e, 0, 0 );
+ return( e );
}
/*
More information about the Fedora-directory-commits
mailing list