Avoid double free in ldap modify requests. The values received in the

modify request is linked into the stored ber structure, and then both are
freed. Fix this by unlinking the values from the request.

diff --git a/usr.sbin/ldapd/attributes.c b/usr.sbin/ldapd/attributes.c
index c6c724e..966969d 100644 (file)
--- a/usr.sbin/ldapd/attributes.c
+++ b/usr.sbin/ldapd/attributes.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: attributes.c,v 1.1 2010/05/31 17:36:31 martinh Exp $ */
+/*     $OpenBSD: attributes.c,v 1.2 2010/07/13 12:54:51 martinh Exp $ */
 
 /*
  * Copyright (c) 2009 Martin Hedenfalk <martin@bzero.se>
@@ -132,6 +132,9 @@ ldap_set_values(struct ber_element *elm, struct ber_element *vals)
        old_vals->be_sub = NULL;
        ber_link_elements(old_vals, vals->be_sub);
 
+       vals->be_sub = NULL;
+       ber_free_elements(vals);
+
        return 0;
 }
 
@@ -157,6 +160,9 @@ ldap_merge_values(struct ber_element *elm, struct ber_element *vals)
 
        ber_link_elements(last, vals->be_sub);
 
+       vals->be_sub = NULL;
+       ber_free_elements(vals);
+
        return 0;
 }
 

diff --git a/usr.sbin/ldapd/modify.c b/usr.sbin/ldapd/modify.c
index 5487ae7..61c168b 100644 (file)
--- a/usr.sbin/ldapd/modify.c
+++ b/usr.sbin/ldapd/modify.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: modify.c,v 1.12 2010/07/06 20:02:33 martinh Exp $ */
+/*     $OpenBSD: modify.c,v 1.13 2010/07/13 12:54:51 martinh Exp $ */
 
 /*
  * Copyright (c) 2009, 2010 Martin Hedenfalk <martin@bzero.se>
@@ -216,7 +216,7 @@ ldap_modify(struct request *req)
        char                    *dn;
        long long                op;
        char                    *attr;
-       struct ber_element      *mods, *entry, *mod, *vals, *a, *set;
+       struct ber_element      *mods, *entry, *mod, *vals, *a, *set, *prev = NULL;
        struct namespace        *ns;
        struct attr_type        *at;
        struct referrals        *refs;
@@ -258,11 +258,14 @@ ldap_modify(struct request *req)
        }
 
        for (mod = mods->be_sub; mod; mod = mod->be_next) {
-               if (ber_scanf_elements(mod, "{E{se", &op, &attr, &vals) != 0) {
+               if (ber_scanf_elements(mod, "{E{ese", &op, &prev, &attr, &vals) != 0) {
                        rc = LDAP_PROTOCOL_ERROR;
+                       vals = NULL;
                        goto done;
                }
 
+               prev->be_next = NULL;
+
                if (!ns->relax) {
                        at = lookup_attribute(conf->schema, attr);
                        if (at == NULL) {
@@ -282,10 +285,13 @@ ldap_modify(struct request *req)
 
                switch (op) {
                case LDAP_MOD_ADD:
-                       if (a == NULL)
-                               ldap_add_attribute(entry, attr, vals);
-                       else
-                               ldap_merge_values(a, vals);
+                       if (a == NULL) {
+                               if (ldap_add_attribute(entry, attr, vals) != NULL)
+                                       vals = NULL;
+                       } else {
+                               if (ldap_merge_values(a, vals->be_sub) == 0)
+                                       vals = NULL;
+                       }
                        break;
                case LDAP_MOD_DELETE:
                        if (vals->be_sub &&
@@ -295,15 +301,23 @@ ldap_modify(struct request *req)
                                ldap_del_attribute(entry, attr);
                        break;
                case LDAP_MOD_REPLACE:
-                       if (vals->be_sub) {
-                               if (a == NULL)
-                                       ldap_add_attribute(entry, attr, vals);
-                               else
-                                       ldap_set_values(a, vals);
+                       if (vals->be_sub != NULL) {
+                               if (a == NULL) {
+                                       if (ldap_add_attribute(entry, attr, vals) != NULL)
+                                               vals = NULL;
+                               } else {
+                                       if (ldap_set_values(a, vals->be_sub) == 0)
+                                               vals = NULL;
+                               }
                        } else if (a == NULL)
                                ldap_del_attribute(entry, attr);
                        break;
                }
+
+               if (vals != NULL) {
+                       ber_free_elements(vals);
+                       vals = NULL;
+               }
        }
 
        if ((rc = validate_entry(dn, entry, ns->relax)) != LDAP_SUCCESS)
@@ -329,6 +343,8 @@ ldap_modify(struct request *req)
                rc = LDAP_OTHER;
 
 done:
+       if (vals != NULL)
+               ber_free_elements(vals);
        namespace_abort(ns);
        return ldap_respond(req, rc);
 }