Attachment 17317 Details for Bug 15008 – patch for master

[patch] patch for master

bug_15008v3.patch (text/plain), 5.07 KB, created by Jo Sutton on 2022-06-03 04:22:40 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jo Sutton

Created: 2022-06-03 04:22:40 UTC

Size: 5.07 KB

patch

obsolete

>From ebdb9135bcf19056484e89e5d7551f436883383d Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Wed, 16 Feb 2022 17:03:10 +1300
>Subject: [PATCH 1/4] s4/dsdb/samldb: Check for empty values array
>
>This avoids potentially trying to access the first element of an empty
>array.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15008
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> source4/dsdb/samdb/ldb_modules/samldb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
>index 24971d521aa..116c3ec1f00 100644
>--- a/source4/dsdb/samdb/ldb_modules/samldb.c
>+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
>@@ -751,7 +751,7 @@ static int samldb_schema_add_handle_linkid(struct samldb_ctx *ac)
> 		return ret;
> 	}
> 
>-	if (el == NULL) {
>+	if (el == NULL || el->num_values == 0) {
> 		return LDB_SUCCESS;
> 	}
> 
>@@ -919,7 +919,7 @@ static int samldb_schema_add_handle_mapiid(struct samldb_ctx *ac)
> 		return ret;
> 	}
> 
>-	if (el == NULL) {
>+	if (el == NULL || el->num_values == 0) {
> 		return LDB_SUCCESS;
> 	}
> 
>-- 
>2.35.0
>
>
>From ad5099332f959f28b34dabc9205e75680016e6e4 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Thu, 17 Feb 2022 11:11:53 +1300
>Subject: [PATCH 2/4] s4/dsdb/util: Use correct value for loop count limit
>
>Currently, we can crash the server by sending a large number of values
>of a specific attribute (such as sAMAccountName) spread across a few
>message elements. If val_count is larger than the total number of
>elements, we get an access beyond the elements array.
>
>Similarly, we can include unrelated message elements prior to the
>message elements of the attribute in question, so that not all of the
>attribute's values are copied into the returned elements values array.
>This can cause the server to access uninitialised data, likely resulting
>in a crash or unexpected behaviour.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15008
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> source4/dsdb/samdb/ldb_modules/util.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/util.c b/source4/dsdb/samdb/ldb_modules/util.c
>index 405febf0b3d..14947746837 100644
>--- a/source4/dsdb/samdb/ldb_modules/util.c
>+++ b/source4/dsdb/samdb/ldb_modules/util.c
>@@ -1546,7 +1546,7 @@ int dsdb_get_expected_new_values(TALLOC_CTX *mem_ctx,
> 
> 	v = _el->values;
> 
>-	for (i = 0; i < val_count; i++) {
>+	for (i = 0; i < msg->num_elements; i++) {
> 		if (ldb_attr_cmp(msg->elements[i].name, attr_name) == 0) {
> 			if ((operation == LDB_MODIFY) &&
> 			    (LDB_FLAG_MOD_TYPE(msg->elements[i].flags)
>-- 
>2.35.0
>
>
>From 2b1a31460f1cae4772f5622c137c6f51f3778cc8 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Thu, 17 Feb 2022 11:13:38 +1300
>Subject: [PATCH 3/4] s4/dsdb/util: Don't call memcpy() with a NULL pointer
>
>Doing so is undefined behaviour.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15008
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> source4/dsdb/samdb/ldb_modules/util.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/util.c b/source4/dsdb/samdb/ldb_modules/util.c
>index 14947746837..35ae110b5ef 100644
>--- a/source4/dsdb/samdb/ldb_modules/util.c
>+++ b/source4/dsdb/samdb/ldb_modules/util.c
>@@ -1548,15 +1548,19 @@ int dsdb_get_expected_new_values(TALLOC_CTX *mem_ctx,
> 
> 	for (i = 0; i < msg->num_elements; i++) {
> 		if (ldb_attr_cmp(msg->elements[i].name, attr_name) == 0) {
>+			const struct ldb_message_element *tmp_el = &msg->elements[i];
> 			if ((operation == LDB_MODIFY) &&
>-			    (LDB_FLAG_MOD_TYPE(msg->elements[i].flags)
>+			    (LDB_FLAG_MOD_TYPE(tmp_el->flags)
> 						== LDB_FLAG_MOD_DELETE)) {
> 				continue;
> 			}
>+			if (tmp_el->values == NULL || tmp_el->num_values == 0) {
>+				continue;
>+			}
> 			memcpy(v,
>-			       msg->elements[i].values,
>-			       msg->elements[i].num_values);
>-			v += msg->elements[i].num_values;
>+			       tmp_el->values,
>+			       tmp_el->num_values);
>+			v += tmp_el->num_values;
> 		}
> 	}
> 
>-- 
>2.35.0
>
>
>From 821ee9a90d5acaca2c785aa790c8ade88a7267c4 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Fri, 3 Jun 2022 16:16:31 +1200
>Subject: [PATCH 4/4] s4/dsdb/util: Correctly copy values into message element
>
>To use memcpy(), we need to specify the number of bytes to copy, rather
>than the number of ldb_val structures.
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> source4/dsdb/samdb/ldb_modules/util.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/util.c b/source4/dsdb/samdb/ldb_modules/util.c
>index 35ae110b5ef..e7fe8f855df 100644
>--- a/source4/dsdb/samdb/ldb_modules/util.c
>+++ b/source4/dsdb/samdb/ldb_modules/util.c
>@@ -1559,7 +1559,7 @@ int dsdb_get_expected_new_values(TALLOC_CTX *mem_ctx,
> 			}
> 			memcpy(v,
> 			       tmp_el->values,
>-			       tmp_el->num_values);
>+			       tmp_el->num_values * sizeof(*v));
> 			v += tmp_el->num_values;
> 		}
> 	}
>-- 
>2.35.0
>

Actions: View

Attachments on bug 15008: 17203 | 17204 | 17317 | 17340 | 17341 | 17343 | 17379