From 0ad5d41ecc438d933791974108fc2797cf5522da Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 31 May 2023 15:34:26 +0200 Subject: [PATCH 1/2] CVE-2023-34966: CI: test for sl_unpack_loop() Send a maliciously crafted packet where a nil type has a subcount of 0. This triggers an endless loop in mdssvc sl_unpack_loop(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=15340 Signed-off-by: Ralph Boehme --- source4/torture/rpc/mdssvc.c | 100 +++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/source4/torture/rpc/mdssvc.c b/source4/torture/rpc/mdssvc.c index 8f16af664766..d0a2d33cf9e3 100644 --- a/source4/torture/rpc/mdssvc.c +++ b/source4/torture/rpc/mdssvc.c @@ -569,6 +569,102 @@ static bool test_mdssvc_invalid_ph_cmd(struct torture_context *tctx, return ok; } +static uint8_t test_sl_unpack_loop_buf[] = { + 0x34, 0x33, 0x32, 0x31, 0x33, 0x30, 0x64, 0x6d, + 0x1d, 0x00, 0x00, 0x00, 0x16, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x02, 0x01, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x02, 0x02, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x02, 0x03, 0x00, 0x00, 0x00, + 0x06, 0x00, 0x00, 0x07, 0x04, 0x00, 0x00, 0x00, + 0x66, 0x65, 0x74, 0x63, 0x68, 0x41, 0x74, 0x74, + 0x72, 0x69, 0x62, 0x75, 0x74, 0x65, 0x73, 0x3a, + 0x66, 0x6f, 0x72, 0x4f, 0x49, 0x44, 0x41, 0x72, + 0x72, 0x61, 0x79, 0x3a, 0x63, 0x6f, 0x6e, 0x74, + 0x65, 0x78, 0x74, 0x3a, 0x00, 0x00, 0x00, 0xea, + 0x02, 0x00, 0x00, 0x84, 0x02, 0x00, 0x00, 0x00, + 0x0a, 0x50, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x02, 0x04, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x02, 0x05, 0x00, 0x00, 0x00, + 0x03, 0x00, 0x00, 0x07, 0x03, 0x00, 0x00, 0x00, + 0x6b, 0x4d, 0x44, 0x49, 0x74, 0x65, 0x6d, 0x50, + 0x61, 0x74, 0x68, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x02, 0x06, 0x00, 0x00, 0x00, + 0x03, 0x00, 0x00, 0x87, 0x08, 0x00, 0x00, 0x00, + 0x01, 0x00, 0xdd, 0x0a, 0x20, 0x00, 0x00, 0x6b, + 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x07, 0x00, 0x00, 0x88, 0x00, 0x00, 0x00, 0x00, + 0x02, 0x00, 0x00, 0x0a, 0x03, 0x00, 0x00, 0x00, + 0x03, 0x00, 0x00, 0x0a, 0x03, 0x00, 0x00, 0x00, + 0x04, 0x00, 0x00, 0x0c, 0x04, 0x00, 0x00, 0x00, + 0x0e, 0x00, 0x00, 0x0a, 0x01, 0x00, 0x00, 0x00, + 0x0f, 0x00, 0x00, 0x0c, 0x03, 0x00, 0x00, 0x00, + 0x13, 0x00, 0x00, 0x1a, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00 +}; + +static bool test_mdssvc_sl_unpack_loop(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; + uint32_t device_id; + uint32_t unkn2; + uint32_t unkn9; + uint32_t fragment; + uint32_t flags; + NTSTATUS status; + bool ok = true; + + device_id = UINT32_C(0x2f000045); + unkn2 = 23; + unkn9 = 0; + fragment = 0; + flags = UINT32_C(0x6b000001); + + request_blob.spotlight_blob = test_sl_unpack_loop_buf; + request_blob.size = sizeof(test_sl_unpack_loop_buf); + request_blob.length = sizeof(test_sl_unpack_loop_buf); + + 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_unknown1 failed\n"); + +done: + return ok; +} + static bool test_mdssvc_invalid_ph_close(struct torture_context *tctx, void *data) { @@ -840,5 +936,9 @@ struct torture_suite *torture_rpc_mdssvc(TALLOC_CTX *mem_ctx) "fetch_unknown_cnid", test_mdssvc_fetch_attr_unknown_cnid); + torture_tcase_add_simple_test(tcase, + "mdssvc_sl_unpack_loop", + test_mdssvc_sl_unpack_loop); + return suite; } -- 2.40.0 From bca354e2cd7d5ccd90c145e19d62ed4b6806c459 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 26 May 2023 13:06:19 +0200 Subject: [PATCH 2/2] CVE-2023-34966: mdssvc: harden sl_unpack_loop() A malicious client could send a packet where subcount is zero, leading to a busy loop because count -= subcount => count -= 0 => while (count > 0) loops forever. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15340 Signed-off-by: Ralph Boehme --- source3/rpc_server/mdssvc/marshalling.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/source3/rpc_server/mdssvc/marshalling.c b/source3/rpc_server/mdssvc/marshalling.c index 9ba6ef571f24..d794ba158389 100644 --- a/source3/rpc_server/mdssvc/marshalling.c +++ b/source3/rpc_server/mdssvc/marshalling.c @@ -1119,7 +1119,7 @@ static ssize_t sl_unpack_loop(DALLOC_CTX *query, sl_nil_t nil = 0; subcount = tag.count; - if (subcount > count) { + if (subcount < 1 || subcount > count) { return -1; } for (i = 0; i < subcount; i++) { @@ -1147,7 +1147,7 @@ static ssize_t sl_unpack_loop(DALLOC_CTX *query, case SQ_TYPE_INT64: subcount = sl_unpack_ints(query, buf, offset, bufsize, encoding); - if (subcount == -1 || subcount > count) { + if (subcount < 1 || subcount > count) { return -1; } offset += tag.size; @@ -1156,7 +1156,7 @@ static ssize_t sl_unpack_loop(DALLOC_CTX *query, case SQ_TYPE_UUID: subcount = sl_unpack_uuid(query, buf, offset, bufsize, encoding); - if (subcount == -1 || subcount > count) { + if (subcount < 1 || subcount > count) { return -1; } offset += tag.size; @@ -1165,7 +1165,7 @@ static ssize_t sl_unpack_loop(DALLOC_CTX *query, case SQ_TYPE_FLOAT: subcount = sl_unpack_floats(query, buf, offset, bufsize, encoding); - if (subcount == -1 || subcount > count) { + if (subcount < 1 || subcount > count) { return -1; } offset += tag.size; @@ -1174,7 +1174,7 @@ static ssize_t sl_unpack_loop(DALLOC_CTX *query, case SQ_TYPE_DATE: subcount = sl_unpack_date(query, buf, offset, bufsize, encoding); - if (subcount == -1 || subcount > count) { + if (subcount < 1 || subcount > count) { return -1; } offset += tag.size; -- 2.40.0