From f12396bd54a8fefffac811cdb33f6dbe7b93ad60 Mon Sep 17 00:00:00 2001
From: martinh <martinh@openbsd.org>
Date: Tue, 13 Jul 2010 12:54:51 +0000
Subject: [PATCH] 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.

---
 usr.sbin/ldapd/attributes.c |  8 +++++++-
 usr.sbin/ldapd/modify.c     | 40 ++++++++++++++++++++++++++-----------
 2 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/usr.sbin/ldapd/attributes.c b/usr.sbin/ldapd/attributes.c
index c6c724ea200..966969d5cbb 100644
--- 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 5487ae78480..61c168b1cdf 100644
--- 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);
 }
-- 
2.20.1