From 5cf13e59d13caf9f37e28e4beabdb1a1d955b08b Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <ab@samba.org>
Date: Fri, 11 Jan 2013 13:56:34 +0200
Subject: [PATCH] Fix CVE-2013-0172

Rebase for 4.0.0 alpha18:
o  CVE-2013-0172:
   Samba 4.0.0 as an AD DC may provide authenticated users with write access
   to LDAP directory objects.

   In AD, Access Control Entries can be assigned based on the objectClass
   of the object.  If a user or a group the user is a member of has any
   access based on the objectClass, then that user has write access to that
   object.

   Additionally, if a user has write access to any attribute on the object,
   they may have access to write to all attributes.

   An important mitigation is that anonymous access is totally disabled by
   default.  The second important mitigation is that normal users are
   typically only given the problematic per-objectClass right via the
   "pre-windows 2000 compatible access" group, and Samba 4.0.0 incorrectly
   does not make "authenticated users" part of this group.
---
 libcli/security/object_tree.c        |  1 +
 source4/dsdb/samdb/ldb_modules/acl.c | 55 ++++++++++++++++++------------------
 source4/dsdb/tests/python/acl.py     | 15 ++++++++++
 3 files changed, 43 insertions(+), 28 deletions(-)

diff --git a/libcli/security/object_tree.c b/libcli/security/object_tree.c
index 6809c8e3452d85312eaf5ede98b24b641abd2cd6..dcbd310baeaad7e7c12a89445fe12a1c125bc9a8 100644
--- a/libcli/security/object_tree.c
+++ b/libcli/security/object_tree.c
@@ -53,6 +53,7 @@ bool insert_in_object_tree(TALLOC_CTX *mem_ctx,
 			return false;
 		}
 		(*root)->guid = *guid;
+		(*root)->remaining_access = init_access;
 		*new_node = *root;
 		return true;
 	}
diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c
index abde85f682c65b633b630ce4f3cec90608501af3..da75789bce11eef6b8e4b8645fd498fa495abf42 100644
--- a/source4/dsdb/samdb/ldb_modules/acl.c
+++ b/source4/dsdb/samdb/ldb_modules/acl.c
@@ -871,8 +871,6 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req)
 	unsigned int i;
 	const struct GUID *guid;
 	uint32_t access_granted;
-	struct object_tree *root = NULL;
-	struct object_tree *new_node = NULL;
 	NTSTATUS status;
 	struct ldb_result *acl_res;
 	struct security_descriptor *sd;
@@ -935,12 +933,6 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req)
 				 "acl_modify: Error retrieving object class GUID.");
 	}
 	sid = samdb_result_dom_sid(req, acl_res->msgs[0], "objectSid");
-	if (!insert_in_object_tree(tmp_ctx, guid, SEC_ADS_WRITE_PROP,
-				   &root, &new_node)) {
-		talloc_free(tmp_ctx);
-		return ldb_error(ldb, LDB_ERR_OPERATIONS_ERROR,
-				 "acl_modify: Error adding new node in object tree.");
-	}
 	for (i=0; i < req->op.mod.message->num_elements; i++){
 		const struct dsdb_attribute *attr;
 		attr = dsdb_attribute_by_lDAPDisplayName(schema,
@@ -1008,6 +1000,8 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req)
 				goto fail;
 			}
 		} else {
+			struct object_tree *root = NULL;
+			struct object_tree *new_node = NULL;
 
 		/* This basic attribute existence check with the right errorcode
 		 * is needed since this module is the first one which requests
@@ -1022,6 +1016,14 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req)
 				ret =  LDB_ERR_NO_SUCH_ATTRIBUTE;
 				goto fail;
 			}
+
+			if (!insert_in_object_tree(tmp_ctx, guid, SEC_ADS_WRITE_PROP,
+						   &root, &new_node)) {
+				talloc_free(tmp_ctx);
+				return ldb_error(ldb, LDB_ERR_OPERATIONS_ERROR,
+						 "acl_modify: Error adding new node in object tree.");
+			}
+
 			if (!insert_in_object_tree(tmp_ctx,
 						   &attr->attributeSecurityGUID, SEC_ADS_WRITE_PROP,
 						   &new_node, &new_node)) {
@@ -1038,27 +1040,24 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req)
 				ret = LDB_ERR_OPERATIONS_ERROR;
 				goto fail;
 			}
-		}
-	}
 
-	if (root->num_of_children > 0) {
-		status = sec_access_check_ds(sd, acl_user_token(module),
-					     SEC_ADS_WRITE_PROP,
-					     &access_granted,
-					     root,
-					     sid);
-
-		if (!NT_STATUS_IS_OK(status)) {
-			ldb_asprintf_errstring(ldb_module_get_ctx(module),
-					       "Object %s has no write property access\n",
-					       ldb_dn_get_linearized(req->op.mod.message->dn));
-			dsdb_acl_debug(sd,
-				       acl_user_token(module),
-				       req->op.mod.message->dn,
-				       true,
-				       10);
-			ret = LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS;
-			goto fail;
+			status = sec_access_check_ds(sd, acl_user_token(module),
+						     SEC_ADS_WRITE_PROP,
+						     &access_granted,
+						     root,
+						     sid);
+			if (!NT_STATUS_IS_OK(status)) {
+				ldb_asprintf_errstring(ldb_module_get_ctx(module),
+						       "Object %s has no write property access\n",
+						       ldb_dn_get_linearized(req->op.mod.message->dn));
+				dsdb_acl_debug(sd,
+					       acl_user_token(module),
+					       req->op.mod.message->dn,
+					       true,
+					       10);
+				ret = LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS;
+				goto fail;
+			}
 		}
 	}
 
diff --git a/source4/dsdb/tests/python/acl.py b/source4/dsdb/tests/python/acl.py
index a62461449d9884b841855daca79a8f9f674865cb..8c82b3c991c523c878ef47e8b985e459691a4398 100755
--- a/source4/dsdb/tests/python/acl.py
+++ b/source4/dsdb/tests/python/acl.py
@@ -389,6 +389,21 @@ url: www.samba.org"""
         else:
             # This 'modify' operation should always throw ERR_INSUFFICIENT_ACCESS_RIGHTS
             self.fail()
+        # Modify on attribute you do not have rights for granted while also modifying something you do have rights for
+        ldif = """
+dn: CN=test_modify_group1,CN=Users,""" + self.base_dn + """
+changetype: modify
+replace: url
+url: www.samba.org
+replace: displayName
+displayName: test_changed"""
+        try:
+            self.ldb_user.modify_ldif(ldif)
+        except LdbError, (num, _):
+            self.assertEquals(num, ERR_INSUFFICIENT_ACCESS_RIGHTS)
+        else:
+            # This 'modify' operation should always throw ERR_INSUFFICIENT_ACCESS_RIGHTS
+            self.fail()
         # Second test object -- Organizational Unit
         print "Testing modify on OU object"
         self.ldb_admin.create_ou("OU=test_modify_ou1," + self.base_dn)
-- 
1.8.1