Attachment 17944 Details for Bug 15341 – Patch for 4.17

[patch] Patch for 4.17

CVE-2023-34967-type_checking-4.17-01.patch (text/plain), 8.55 KB, created by Ralph Böhme on 2023-06-23 14:18:53 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralph Böhme

Created: 2023-06-23 14:18:53 UTC

Size: 8.55 KB

patch

obsolete

>From d8a4de972fe529e2c324a5abfb465e1812961176 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Wed, 31 May 2023 16:26:14 +0200
>Subject: [PATCH 1/2] CVE-2023-34967: CI: add a test for type checking of
> dalloc_value_for_key()
>
>Sends a maliciously crafted packet where the value in a key/value style
>dictionary for the "scope" key is a simple string object whereas the server
>expects an array. As the server doesn't perform type validation on the value, it
>crashes when trying to use the "simple" object as a "complex" one.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15341
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>---
> source4/torture/rpc/mdssvc.c | 134 +++++++++++++++++++++++++++++++++++
> 1 file changed, 134 insertions(+)
>
>diff --git a/source4/torture/rpc/mdssvc.c b/source4/torture/rpc/mdssvc.c
>index d0a2d33cf9e3..3689692f7de0 100644
>--- a/source4/torture/rpc/mdssvc.c
>+++ b/source4/torture/rpc/mdssvc.c
>@@ -665,6 +665,136 @@ static bool test_mdssvc_sl_unpack_loop(struct torture_context *tctx,
> 	return ok;
> }
> 
>+static bool test_sl_dict_type_safety(struct torture_context *tctx,
>+				     void *data)
>+{
>+	struct torture_mdsscv_state *state = talloc_get_type_abort(
>+		data, struct torture_mdsscv_state);
>+	struct dcerpc_binding_handle *b = state->p->binding_handle;
>+	struct mdssvc_blob request_blob;
>+	struct mdssvc_blob response_blob;
>+	uint64_t ctx1 = 0xdeadbeef;
>+	uint64_t ctx2 = 0xcafebabe;
>+	uint32_t device_id;
>+	uint32_t unkn2;
>+	uint32_t unkn9;
>+	uint32_t fragment;
>+	uint32_t flags;
>+	DALLOC_CTX *d = NULL;
>+	sl_array_t *array1 = NULL, *array2 = NULL;
>+	sl_dict_t *arg = NULL;
>+	int result;
>+	NTSTATUS status;
>+	bool ok = true;
>+
>+	device_id = UINT32_C(0x2f000045);
>+	unkn2 = 23;
>+	unkn9 = 0;
>+	fragment = 0;
>+	flags = UINT32_C(0x6b000001);
>+
>+	d = dalloc_new(tctx);
>+	torture_assert_not_null_goto(tctx, d,
>+				     ok, done, "dalloc_new failed\n");
>+
>+	array1 = dalloc_zero(d, sl_array_t);
>+	torture_assert_not_null_goto(tctx, array1,
>+				     ok, done, "dalloc_zero failed\n");
>+
>+	array2 = dalloc_zero(d, sl_array_t);
>+	torture_assert_not_null_goto(tctx, array2,
>+				     ok, done, "dalloc_new failed\n");
>+
>+	result = dalloc_stradd(array2, "openQueryWithParams:forContext:");
>+	torture_assert_goto(tctx, result == 0,
>+			    ok, done, "dalloc_stradd failed\n");
>+
>+	result = dalloc_add_copy(array2, &ctx1, uint64_t);
>+	torture_assert_goto(tctx, result == 0,
>+			    ok, done, "dalloc_stradd failed\n");
>+
>+	result = dalloc_add_copy(array2, &ctx2, uint64_t);
>+	torture_assert_goto(tctx, result == 0,
>+			    ok, done, "dalloc_stradd failed\n");
>+
>+	arg = dalloc_zero(array1, sl_dict_t);
>+	torture_assert_not_null_goto(tctx, d,
>+				     ok, done, "dalloc_zero failed\n");
>+
>+	result = dalloc_stradd(arg, "kMDQueryString");
>+	torture_assert_goto(tctx, result == 0,
>+			    ok, done, "dalloc_stradd failed\n");
>+
>+	result = dalloc_stradd(arg, "*");
>+	torture_assert_goto(tctx, result == 0,
>+			    ok, done, "dalloc_stradd failed\n");
>+
>+	result = dalloc_stradd(arg, "kMDScopeArray");
>+	torture_assert_goto(tctx, result == 0,
>+			    ok, done, "dalloc_stradd failed\n");
>+
>+	result = dalloc_stradd(arg, "AAAABBBB");
>+	torture_assert_goto(tctx, result == 0,
>+			    ok, done, "dalloc_stradd failed\n");
>+
>+	result = dalloc_add(array1, array2, sl_array_t);
>+	torture_assert_goto(tctx, result == 0,
>+			    ok, done, "dalloc_add failed\n");
>+
>+	result = dalloc_add(array1, arg, sl_dict_t);
>+	torture_assert_goto(tctx, result == 0,
>+			    ok, done, "dalloc_add failed\n");
>+
>+	result = dalloc_add(d, array1, sl_array_t);
>+	torture_assert_goto(tctx, result == 0,
>+			    ok, done, "dalloc_add failed\n");
>+
>+	torture_comment(tctx, "%s", dalloc_dump(d, 0));
>+
>+	request_blob.spotlight_blob = talloc_array(tctx,
>+						   uint8_t,
>+						   64 * 1024);
>+	torture_assert_not_null_goto(tctx, request_blob.spotlight_blob,
>+				     ok, done, "dalloc_new failed\n");
>+	request_blob.size = 64 * 1024;
>+
>+	request_blob.length = sl_pack(d,
>+				      (char *)request_blob.spotlight_blob,
>+				      request_blob.size);
>+	torture_assert_goto(tctx, request_blob.length > 0,
>+			    ok, done, "sl_pack failed\n");
>+
>+	response_blob.spotlight_blob = talloc_array(state, uint8_t, 0);
>+	torture_assert_not_null_goto(tctx, response_blob.spotlight_blob,
>+				     ok, done, "dalloc_zero failed\n");
>+	response_blob.size = 0;
>+
>+	status = dcerpc_mdssvc_cmd(b,
>+				   state,
>+				   &state->ph,
>+				   0,
>+				   device_id,
>+				   unkn2,
>+				   0,
>+				   flags,
>+				   request_blob,
>+				   0,
>+				   64 * 1024,
>+				   1,
>+				   64 * 1024,
>+				   0,
>+				   0,
>+				   &fragment,
>+				   &response_blob,
>+				   &unkn9);
>+	torture_assert_ntstatus_ok_goto(
>+		tctx, status, ok, done,
>+		"dcerpc_mdssvc_cmd failed\n");
>+
>+done:
>+	return ok;
>+}
>+
> static bool test_mdssvc_invalid_ph_close(struct torture_context *tctx,
> 					 void *data)
> {
>@@ -940,5 +1070,9 @@ struct torture_suite *torture_rpc_mdssvc(TALLOC_CTX *mem_ctx)
> 				      "mdssvc_sl_unpack_loop",
> 				      test_mdssvc_sl_unpack_loop);
> 
>+	torture_tcase_add_simple_test(tcase,
>+				      "sl_dict_type_safety",
>+				      test_sl_dict_type_safety);
>+
> 	return suite;
> }
>-- 
>2.40.0
>
>
>From 846b526a74c2b0a9ad250f9b60d6e234799c1f02 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Fri, 26 May 2023 15:06:38 +0200
>Subject: [PATCH 2/2] CVE-2023-34967: mdssvc: add type checking to
> dalloc_value_for_key()
>
>Change the dalloc_value_for_key() function to require an additional final
>argument which denotes the expected type of the value associated with a key. If
>the types don't match, return NULL.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15341
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>---
> source3/rpc_server/mdssvc/dalloc.c | 14 ++++++++++----
> source3/rpc_server/mdssvc/mdssvc.c | 17 +++++++++++++----
> 2 files changed, 23 insertions(+), 8 deletions(-)
>
>diff --git a/source3/rpc_server/mdssvc/dalloc.c b/source3/rpc_server/mdssvc/dalloc.c
>index 007702d45408..8b79b41fd975 100644
>--- a/source3/rpc_server/mdssvc/dalloc.c
>+++ b/source3/rpc_server/mdssvc/dalloc.c
>@@ -159,7 +159,7 @@ void *dalloc_value_for_key(const DALLOC_CTX *d, ...)
> 	int result = 0;
> 	void *p = NULL;
> 	va_list args;
>-	const char *type;
>+	const char *type = NULL;
> 	int elem;
> 	size_t array_len;
> 
>@@ -170,7 +170,6 @@ void *dalloc_value_for_key(const DALLOC_CTX *d, ...)
> 		array_len = talloc_array_length(d->dd_talloc_array);
> 		elem = va_arg(args, int);
> 		if (elem >= array_len) {
>-			va_end(args);
> 			result = -1;
> 			goto done;
> 		}
>@@ -178,8 +177,6 @@ void *dalloc_value_for_key(const DALLOC_CTX *d, ...)
> 		type = va_arg(args, const char *);
> 	}
> 
>-	va_end(args);
>-
> 	array_len = talloc_array_length(d->dd_talloc_array);
> 
> 	for (elem = 0; elem + 1 < array_len; elem += 2) {
>@@ -192,8 +189,17 @@ void *dalloc_value_for_key(const DALLOC_CTX *d, ...)
> 			break;
> 		}
> 	}
>+	if (p == NULL) {
>+		goto done;
>+	}
>+
>+	type = va_arg(args, const char *);
>+	if (strcmp(talloc_get_name(p), type) != 0) {
>+		p = NULL;
>+	}
> 
> done:
>+	va_end(args);
> 	if (result != 0) {
> 		p = NULL;
> 	}
>diff --git a/source3/rpc_server/mdssvc/mdssvc.c b/source3/rpc_server/mdssvc/mdssvc.c
>index 9b32c99b8b3a..7dd3c84713f1 100644
>--- a/source3/rpc_server/mdssvc/mdssvc.c
>+++ b/source3/rpc_server/mdssvc/mdssvc.c
>@@ -872,7 +872,8 @@ static bool slrpc_open_query(struct mds_ctx *mds_ctx,
> 
> 	querystring = dalloc_value_for_key(query, "DALLOC_CTX", 0,
> 					   "DALLOC_CTX", 1,
>-					   "kMDQueryString");
>+					   "kMDQueryString",
>+					   "char *");
> 	if (querystring == NULL) {
> 		DEBUG(1, ("missing kMDQueryString\n"));
> 		goto error;
>@@ -912,8 +913,11 @@ static bool slrpc_open_query(struct mds_ctx *mds_ctx,
> 	slq->ctx2 = *uint64p;
> 
> 	path_scope = dalloc_value_for_key(query, "DALLOC_CTX", 0,
>-					  "DALLOC_CTX", 1, "kMDScopeArray");
>+					  "DALLOC_CTX", 1,
>+					  "kMDScopeArray",
>+					  "sl_array_t");
> 	if (path_scope == NULL) {
>+		DBG_ERR("missing kMDScopeArray\n");
> 		goto error;
> 	}
> 
>@@ -934,8 +938,11 @@ static bool slrpc_open_query(struct mds_ctx *mds_ctx,
> 	}
> 
> 	reqinfo = dalloc_value_for_key(query, "DALLOC_CTX", 0,
>-				       "DALLOC_CTX", 1, "kMDAttributeArray");
>+				       "DALLOC_CTX", 1,
>+				       "kMDAttributeArray",
>+				       "sl_array_t");
> 	if (reqinfo == NULL) {
>+		DBG_ERR("missing kMDAttributeArray\n");
> 		goto error;
> 	}
> 
>@@ -943,7 +950,9 @@ static bool slrpc_open_query(struct mds_ctx *mds_ctx,
> 	DEBUG(10, ("requested attributes: %s", dalloc_dump(reqinfo, 0)));
> 
> 	cnids = dalloc_value_for_key(query, "DALLOC_CTX", 0,
>-				     "DALLOC_CTX", 1, "kMDQueryItemArray");
>+				     "DALLOC_CTX", 1,
>+				     "kMDQueryItemArray",
>+				     "sl_array_t");
> 	if (cnids) {
> 		ok = sort_cnids(slq, cnids->ca_cnids);
> 		if (!ok) {
>-- 
>2.40.0
>

Flags:

metze: review+
slow: review? (jra)
slow: ci-passed+

Actions: View

Attachments on bug 15341: 17841 | 17903 | 17911 | 17917 | 17920 | 17943 | 17944 | 17951