From 1dd3ae281b9d9260859822bbf6891e94c2f86882 Mon Sep 17 00:00:00 2001 From: Samuel Cabrero Date: Thu, 24 Feb 2022 17:48:27 +0100 Subject: [PATCH 01/25] CVE-2022-2127: s3:winbind: Move big NTLMv2 blob checks to parent process The winbindd_dual_pam_auth_crap() function will be converted to a local RPC call handler and it won't receive a winbindd_cli_state struct. Move the checks accessing this struct to the parent. Signed-off-by: Samuel Cabrero Reviewed-by: Jeremy Allison (cherry picked from commit 74a511a8eab72cc82940738a1e20e63e12b81374) --- source3/winbindd/winbindd_pam.c | 12 ------------ source3/winbindd/winbindd_pam_auth_crap.c | 12 ++++++++++++ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c index e600ad27e54..e77673bf173 100644 --- a/source3/winbindd/winbindd_pam.c +++ b/source3/winbindd/winbindd_pam.c @@ -2671,18 +2671,6 @@ enum winbindd_result winbindd_dual_pam_auth_crap(struct winbindd_domain *domain, DEBUG(3, ("[%5lu]: pam auth crap domain: %s user: %s\n", (unsigned long)state->pid, name_domain, name_user)); - if (state->request->data.auth_crap.lm_resp_len > sizeof(state->request->data.auth_crap.lm_resp) - || state->request->data.auth_crap.nt_resp_len > sizeof(state->request->data.auth_crap.nt_resp)) { - if (!(state->request->flags & WBFLAG_BIG_NTLMV2_BLOB) || - state->request->extra_len != state->request->data.auth_crap.nt_resp_len) { - DEBUG(0, ("winbindd_pam_auth_crap: invalid password length %u/%u\n", - state->request->data.auth_crap.lm_resp_len, - state->request->data.auth_crap.nt_resp_len)); - result = NT_STATUS_INVALID_PARAMETER; - goto done; - } - } - lm_resp = data_blob_talloc(state->mem_ctx, state->request->data.auth_crap.lm_resp, state->request->data.auth_crap.lm_resp_len); diff --git a/source3/winbindd/winbindd_pam_auth_crap.c b/source3/winbindd/winbindd_pam_auth_crap.c index a6f13806df9..fdb8120a6fe 100644 --- a/source3/winbindd/winbindd_pam_auth_crap.c +++ b/source3/winbindd/winbindd_pam_auth_crap.c @@ -140,6 +140,18 @@ struct tevent_req *winbindd_pam_auth_crap_send( fstrcpy(request->data.auth_crap.workstation, lp_netbios_name()); } + if (request->data.auth_crap.lm_resp_len > sizeof(request->data.auth_crap.lm_resp) + || request->data.auth_crap.nt_resp_len > sizeof(request->data.auth_crap.nt_resp)) { + if (!(request->flags & WBFLAG_BIG_NTLMV2_BLOB) || + request->extra_len != request->data.auth_crap.nt_resp_len) { + DBG_ERR("Invalid password length %u/%u\n", + request->data.auth_crap.lm_resp_len, + request->data.auth_crap.nt_resp_len); + tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); + return tevent_req_post(req, ev); + } + } + subreq = wb_domain_request_send(state, global_event_context(), domain, request); if (tevent_req_nomem(subreq, req)) { -- 2.34.1 From 5c6fe5a491b16bb658c191cfafb5edc0beb5fab2 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Fri, 20 May 2022 10:55:23 +0200 Subject: [PATCH 02/25] CVE-2022-2127: winbindd: Fix WINBINDD_PAM_AUTH_CRAP length checks With WBFLAG_BIG_NTLMV2_BLOB being set plus lm_resp_len too large you can crash winbind. We don't independently check lm_resp_len sufficiently. Discovered via Coverity ID 1504444 Out-of-bounds access BUG: https://bugzilla.samba.org/show_bug.cgi?id=15072 Signed-off-by: Volker Lendecke --- source3/winbindd/winbindd_pam_auth_crap.c | 31 +++++++++++++++-------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/source3/winbindd/winbindd_pam_auth_crap.c b/source3/winbindd/winbindd_pam_auth_crap.c index fdb8120a6fe..651d54b01d3 100644 --- a/source3/winbindd/winbindd_pam_auth_crap.c +++ b/source3/winbindd/winbindd_pam_auth_crap.c @@ -42,6 +42,9 @@ struct tevent_req *winbindd_pam_auth_crap_send( struct winbindd_pam_auth_crap_state *state; struct winbindd_domain *domain; const char *auth_domain = NULL; + bool lmlength_ok = false; + bool ntlength_ok = false; + bool pwlength_ok = false; req = tevent_req_create(mem_ctx, &state, struct winbindd_pam_auth_crap_state); @@ -140,16 +143,24 @@ struct tevent_req *winbindd_pam_auth_crap_send( fstrcpy(request->data.auth_crap.workstation, lp_netbios_name()); } - if (request->data.auth_crap.lm_resp_len > sizeof(request->data.auth_crap.lm_resp) - || request->data.auth_crap.nt_resp_len > sizeof(request->data.auth_crap.nt_resp)) { - if (!(request->flags & WBFLAG_BIG_NTLMV2_BLOB) || - request->extra_len != request->data.auth_crap.nt_resp_len) { - DBG_ERR("Invalid password length %u/%u\n", - request->data.auth_crap.lm_resp_len, - request->data.auth_crap.nt_resp_len); - tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); - return tevent_req_post(req, ev); - } + lmlength_ok = (request->data.auth_crap.lm_resp_len <= + sizeof(request->data.auth_crap.lm_resp)); + + ntlength_ok = (request->data.auth_crap.nt_resp_len <= + sizeof(request->data.auth_crap.nt_resp)); + + ntlength_ok |= + ((request->flags & WBFLAG_BIG_NTLMV2_BLOB) && + (request->extra_len == request->data.auth_crap.nt_resp_len)); + + pwlength_ok = lmlength_ok && ntlength_ok; + + if (!pwlength_ok) { + DBG_ERR("Invalid password length %u/%u\n", + request->data.auth_crap.lm_resp_len, + request->data.auth_crap.nt_resp_len); + tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); + return tevent_req_post(req, ev); } subreq = wb_domain_request_send(state, global_event_context(), domain, -- 2.34.1 From 2eabbe31f64a8456813a502afb05907beb46ffad Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 16 Jun 2023 12:28:47 +0200 Subject: [PATCH 03/25] CVE-2022-2127: ntlm_auth: cap lanman response length value We already copy at most sizeof(request.data.auth_crap.lm_resp) bytes to the lm_resp buffer, but we don't cap the length indicator. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15072 Signed-off-by: Ralph Boehme --- source3/utils/ntlm_auth.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/source3/utils/ntlm_auth.c b/source3/utils/ntlm_auth.c index ceb22e597f7..8ce453f4b92 100644 --- a/source3/utils/ntlm_auth.c +++ b/source3/utils/ntlm_auth.c @@ -576,10 +576,14 @@ NTSTATUS contact_winbind_auth_crap(const char *username, memcpy(request.data.auth_crap.chal, challenge->data, MIN(challenge->length, 8)); if (lm_response && lm_response->length) { + size_t capped_lm_response_len = MIN( + lm_response->length, + sizeof(request.data.auth_crap.lm_resp)); + memcpy(request.data.auth_crap.lm_resp, lm_response->data, - MIN(lm_response->length, sizeof(request.data.auth_crap.lm_resp))); - request.data.auth_crap.lm_resp_len = lm_response->length; + capped_lm_response_len); + request.data.auth_crap.lm_resp_len = capped_lm_response_len; } if (nt_response && nt_response->length) { -- 2.34.1 From 01cf3cf7a83f0f3fcdf1f4026327c84e4c17f853 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 31 May 2023 15:34:26 +0200 Subject: [PATCH 04/25] 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 8f16af66476..d0a2d33cf9e 100644 --- a/source4/torture/rpc/mdssvc.c +++ b/source4/torture/rpc/mdssvc.c @@ -569,6 +569,102 @@ done: 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.34.1 From cb6f3e2202473eeccf81e34ebcdb4bc4f726548a Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 26 May 2023 13:06:19 +0200 Subject: [PATCH 05/25] 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 1aa750413cd..441d41160f1 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.34.1 From 92d014bc44b32478aa597f38bf11687f1fc95ff1 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 31 May 2023 16:26:14 +0200 Subject: [PATCH 06/25] 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 --- 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 d0a2d33cf9e..3689692f7de 100644 --- a/source4/torture/rpc/mdssvc.c +++ b/source4/torture/rpc/mdssvc.c @@ -665,6 +665,136 @@ done: 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.34.1 From 5b4353cc60b75610f0aa12b1cced36d35a4d04d4 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 26 May 2023 15:06:38 +0200 Subject: [PATCH 07/25] 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 --- 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 007702d4540..8b79b41fd97 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 4f1629b2b4d..02c42211694 100644 --- a/source3/rpc_server/mdssvc/mdssvc.c +++ b/source3/rpc_server/mdssvc/mdssvc.c @@ -885,7 +885,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; @@ -925,8 +926,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; } @@ -947,8 +951,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; } @@ -956,7 +963,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.34.1 From 843ec381de3f5ce3740679783d3b3637a7474648 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Sat, 15 Oct 2022 13:29:14 +0200 Subject: [PATCH 08/25] CVE-2023-34968: lib: Move subdir_of() to source3/lib/util_path.c Make it available for other components Bug: https://bugzilla.samba.org/show_bug.cgi?id=15207 Signed-off-by: Volker Lendecke (backported from commit d905dbddf8d2655e6c91752b750cbe9c15837ee5) [slow@samba.org: subdir_of() didn't exist yet in 4.16 so this just adds it] --- source3/lib/util_path.c | 52 +++++++++++++++++++++++++++++++++++++++++ source3/lib/util_path.h | 4 ++++ 2 files changed, 56 insertions(+) diff --git a/source3/lib/util_path.c b/source3/lib/util_path.c index c34b734384c..e6bed724551 100644 --- a/source3/lib/util_path.c +++ b/source3/lib/util_path.c @@ -23,6 +23,8 @@ #include "replace.h" #include +#include "lib/util/debug.h" +#include "lib/util/fault.h" #include "lib/util/samba_util.h" #include "lib/util_path.h" @@ -210,3 +212,53 @@ char *canonicalize_absolute_path(TALLOC_CTX *ctx, const char *pathname_in) *p++ = '\0'; return pathname; } + +/* + * Take two absolute paths, figure out if "subdir" is a proper + * subdirectory of "parent". Return the component relative to the + * "parent" without the potential "/". Take care of "parent" + * possibly ending in "/". + */ +bool subdir_of(const char *parent, + size_t parent_len, + const char *subdir, + const char **_relative) +{ + const char *relative = NULL; + bool matched; + + SMB_ASSERT(parent[0] == '/'); + SMB_ASSERT(subdir[0] == '/'); + + if (parent_len == 1) { + /* + * Everything is below "/" + */ + *_relative = subdir+1; + return true; + } + + if (parent[parent_len-1] == '/') { + parent_len -= 1; + } + + matched = (strncmp(subdir, parent, parent_len) == 0); + if (!matched) { + return false; + } + + relative = &subdir[parent_len]; + + if (relative[0] == '\0') { + *_relative = relative; /* nothing left */ + return true; + } + + if (relative[0] == '/') { + /* End of parent must match a '/' in subdir. */ + *_relative = relative+1; + return true; + } + + return false; +} diff --git a/source3/lib/util_path.h b/source3/lib/util_path.h index 3e7d04de550..0ea508bf5bb 100644 --- a/source3/lib/util_path.h +++ b/source3/lib/util_path.h @@ -31,5 +31,9 @@ char *lock_path(TALLOC_CTX *mem_ctx, const char *name); char *state_path(TALLOC_CTX *mem_ctx, const char *name); char *cache_path(TALLOC_CTX *mem_ctx, const char *name); char *canonicalize_absolute_path(TALLOC_CTX *ctx, const char *abs_path); +bool subdir_of(const char *parent, + size_t parent_len, + const char *subdir, + const char **_relative); #endif -- 2.34.1 From b09e22cfc79845ef751acc9b5ecf479cb56b135f Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 6 Jun 2023 15:17:26 +0200 Subject: [PATCH 09/25] CVE-2023-34968: mdssvc: cache and reuse stat info in struct sl_inode_path_map Prepare for the "path" being a fake path and not the real server-side path where we won't be able to vfs_stat_fsp() this fake path. Luckily we already got stat info for the object in mds_add_result() so we can just pass stat info from there. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15388 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher --- source3/rpc_server/mdssvc/mdssvc.c | 32 +++++++----------------------- source3/rpc_server/mdssvc/mdssvc.h | 1 + 2 files changed, 8 insertions(+), 25 deletions(-) diff --git a/source3/rpc_server/mdssvc/mdssvc.c b/source3/rpc_server/mdssvc/mdssvc.c index 02c42211694..3af0a71a28e 100644 --- a/source3/rpc_server/mdssvc/mdssvc.c +++ b/source3/rpc_server/mdssvc/mdssvc.c @@ -446,7 +446,10 @@ static int ino_path_map_destr_cb(struct sl_inode_path_map *entry) * entries by calling talloc_free() on the query slq handles. **/ -static bool inode_map_add(struct sl_query *slq, uint64_t ino, const char *path) +static bool inode_map_add(struct sl_query *slq, + uint64_t ino, + const char *path, + struct stat_ex *st) { NTSTATUS status; struct sl_inode_path_map *entry; @@ -493,6 +496,7 @@ static bool inode_map_add(struct sl_query *slq, uint64_t ino, const char *path) entry->ino = ino; entry->mds_ctx = slq->mds_ctx; + entry->st = *st; entry->path = talloc_strdup(entry, path); if (entry->path == NULL) { DEBUG(1, ("talloc failed\n")); @@ -630,7 +634,7 @@ bool mds_add_result(struct sl_query *slq, const char *path) return false; } - ok = inode_map_add(slq, ino64, path); + ok = inode_map_add(slq, ino64, path, &sb); if (!ok) { DEBUG(1, ("inode_map_add error\n")); slq->state = SLQ_STATE_ERROR; @@ -1353,29 +1357,7 @@ static bool slrpc_fetch_attributes(struct mds_ctx *mds_ctx, elem = talloc_get_type_abort(p, struct sl_inode_path_map); path = elem->path; - status = synthetic_pathref(talloc_tos(), - mds_ctx->conn->cwd_fsp, - path, - NULL, - NULL, - 0, - 0, - &smb_fname); - if (!NT_STATUS_IS_OK(status)) { - /* This is not an error, the user may lack permissions */ - DBG_DEBUG("synthetic_pathref [%s]: %s\n", - smb_fname_str_dbg(smb_fname), - nt_errstr(status)); - return true; - } - - status = vfs_stat_fsp(smb_fname->fsp); - if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(smb_fname); - return true; - } - - sp = &smb_fname->fsp->fsp_name->st; + sp = &elem->st; } ok = add_filemeta(mds_ctx, reqinfo, fm_array, path, sp); diff --git a/source3/rpc_server/mdssvc/mdssvc.h b/source3/rpc_server/mdssvc/mdssvc.h index 205417c4be1..ff36b329f2b 100644 --- a/source3/rpc_server/mdssvc/mdssvc.h +++ b/source3/rpc_server/mdssvc/mdssvc.h @@ -105,6 +105,7 @@ struct sl_inode_path_map { struct mds_ctx *mds_ctx; uint64_t ino; char *path; + struct stat_ex st; }; /* Per process state */ -- 2.34.1 From 8c95f7ae6b3f30ccdc5ce4f0c44b3f8c1fc6a5c8 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 17 Jun 2023 13:39:55 +0200 Subject: [PATCH 10/25] CVE-2023-34968: mdssvc: add missing "kMDSStoreMetaScopes" dict key in slrpc_fetch_properties() We were adding the value, but not the key. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15388 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher --- source3/rpc_server/mdssvc/mdssvc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source3/rpc_server/mdssvc/mdssvc.c b/source3/rpc_server/mdssvc/mdssvc.c index 3af0a71a28e..72936a99289 100644 --- a/source3/rpc_server/mdssvc/mdssvc.c +++ b/source3/rpc_server/mdssvc/mdssvc.c @@ -743,6 +743,10 @@ static bool slrpc_fetch_properties(struct mds_ctx *mds_ctx, } /* kMDSStoreMetaScopes array */ + result = dalloc_stradd(dict, "kMDSStoreMetaScopes"); + if (result != 0) { + return false; + } array = dalloc_zero(dict, sl_array_t); if (array == NULL) { return NULL; -- 2.34.1 From 3636b54616ee63e17d8571af610a0e21d667b592 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 19 Jun 2023 17:14:38 +0200 Subject: [PATCH 11/25] CVE-2023-34968: mdscli: use correct TALLOC memory context when allocating spotlight_blob d is talloc_free()d at the end of the functions and the buffer was later used after beeing freed in the DCERPC layer when sending the packet. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15388 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher --- source3/rpc_client/cli_mdssvc_util.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source3/rpc_client/cli_mdssvc_util.c b/source3/rpc_client/cli_mdssvc_util.c index fe5092c3790..892a844e71a 100644 --- a/source3/rpc_client/cli_mdssvc_util.c +++ b/source3/rpc_client/cli_mdssvc_util.c @@ -209,7 +209,7 @@ NTSTATUS mdscli_blob_search(TALLOC_CTX *mem_ctx, return NT_STATUS_NO_MEMORY; } - blob->spotlight_blob = talloc_array(d, + blob->spotlight_blob = talloc_array(mem_ctx, uint8_t, ctx->max_fragment_size); if (blob->spotlight_blob == NULL) { @@ -293,7 +293,7 @@ NTSTATUS mdscli_blob_get_results(TALLOC_CTX *mem_ctx, return NT_STATUS_NO_MEMORY; } - blob->spotlight_blob = talloc_array(d, + blob->spotlight_blob = talloc_array(mem_ctx, uint8_t, ctx->max_fragment_size); if (blob->spotlight_blob == NULL) { @@ -426,7 +426,7 @@ NTSTATUS mdscli_blob_get_path(TALLOC_CTX *mem_ctx, return NT_STATUS_NO_MEMORY; } - blob->spotlight_blob = talloc_array(d, + blob->spotlight_blob = talloc_array(mem_ctx, uint8_t, ctx->max_fragment_size); if (blob->spotlight_blob == NULL) { @@ -510,7 +510,7 @@ NTSTATUS mdscli_blob_close_search(TALLOC_CTX *mem_ctx, return NT_STATUS_NO_MEMORY; } - blob->spotlight_blob = talloc_array(d, + blob->spotlight_blob = talloc_array(mem_ctx, uint8_t, ctx->max_fragment_size); if (blob->spotlight_blob == NULL) { -- 2.34.1 From 82cc2a422db8d4402378c2e6f1e138ff385b0f15 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 19 Jun 2023 18:28:41 +0200 Subject: [PATCH 12/25] CVE-2023-34968: mdscli: remove response blob allocation This is handled by the NDR code transparently. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15388 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher --- source3/rpc_client/cli_mdssvc.c | 36 --------------------------------- 1 file changed, 36 deletions(-) diff --git a/source3/rpc_client/cli_mdssvc.c b/source3/rpc_client/cli_mdssvc.c index 82d14372fe4..07c19b51dd4 100644 --- a/source3/rpc_client/cli_mdssvc.c +++ b/source3/rpc_client/cli_mdssvc.c @@ -276,15 +276,6 @@ struct tevent_req *mdscli_search_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } - state->response_blob.spotlight_blob = talloc_array( - state, - uint8_t, - mdscli_ctx->max_fragment_size); - if (tevent_req_nomem(state->response_blob.spotlight_blob, req)) { - return tevent_req_post(req, ev); - } - state->response_blob.size = mdscli_ctx->max_fragment_size; - subreq = dcerpc_mdssvc_cmd_send(state, ev, mdscli_ctx->bh, @@ -457,15 +448,6 @@ struct tevent_req *mdscli_get_results_send( return tevent_req_post(req, ev); } - state->response_blob.spotlight_blob = talloc_array( - state, - uint8_t, - mdscli_ctx->max_fragment_size); - if (tevent_req_nomem(state->response_blob.spotlight_blob, req)) { - return tevent_req_post(req, ev); - } - state->response_blob.size = mdscli_ctx->max_fragment_size; - subreq = dcerpc_mdssvc_cmd_send(state, ev, mdscli_ctx->bh, @@ -681,15 +663,6 @@ struct tevent_req *mdscli_get_path_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } - state->response_blob.spotlight_blob = talloc_array( - state, - uint8_t, - mdscli_ctx->max_fragment_size); - if (tevent_req_nomem(state->response_blob.spotlight_blob, req)) { - return tevent_req_post(req, ev); - } - state->response_blob.size = mdscli_ctx->max_fragment_size; - subreq = dcerpc_mdssvc_cmd_send(state, ev, mdscli_ctx->bh, @@ -852,15 +825,6 @@ struct tevent_req *mdscli_close_search_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } - state->response_blob.spotlight_blob = talloc_array( - state, - uint8_t, - mdscli_ctx->max_fragment_size); - if (tevent_req_nomem(state->response_blob.spotlight_blob, req)) { - return tevent_req_post(req, ev); - } - state->response_blob.size = mdscli_ctx->max_fragment_size; - subreq = dcerpc_mdssvc_cmd_send(state, ev, mdscli_ctx->bh, -- 2.34.1 From 7bbaa191be6a1b389604eff75aba9913b0e75d98 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 20 Jun 2023 11:28:47 +0200 Subject: [PATCH 13/25] CVE-2023-34968: smbtorture: remove response blob allocation in mdssvc.c This is alreay done by NDR for us. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15388 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher --- source4/torture/rpc/mdssvc.c | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/source4/torture/rpc/mdssvc.c b/source4/torture/rpc/mdssvc.c index 3689692f7de..a16bd5b47e3 100644 --- a/source4/torture/rpc/mdssvc.c +++ b/source4/torture/rpc/mdssvc.c @@ -536,13 +536,6 @@ static bool test_mdssvc_invalid_ph_cmd(struct torture_context *tctx, request_blob.length = 0; request_blob.size = 0; - 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, &ph, @@ -632,13 +625,6 @@ static bool test_mdssvc_sl_unpack_loop(struct torture_context *tctx, 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, @@ -764,11 +750,6 @@ static bool test_sl_dict_type_safety(struct torture_context *tctx, 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, @@ -926,13 +907,6 @@ static bool test_mdssvc_fetch_attr_unknown_cnid(struct torture_context *tctx, ret, done, "dalloc_zero failed\n"); request_blob.size = max_fragment_size; - response_blob.spotlight_blob = talloc_array(state, - uint8_t, - max_fragment_size); - torture_assert_not_null_goto(tctx, response_blob.spotlight_blob, - ret, done, "dalloc_zero failed\n"); - response_blob.size = max_fragment_size; - len = sl_pack(d, (char *)request_blob.spotlight_blob, request_blob.size); torture_assert_goto(tctx, len != -1, ret, done, "sl_pack failed\n"); -- 2.34.1 From 739f72a07032da15b3a74a8b96959300d555e836 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 20 Jun 2023 11:35:41 +0200 Subject: [PATCH 14/25] CVE-2023-34968: rpcclient: remove response blob allocation This is alreay done by NDR for us. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15388 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher --- source3/rpcclient/cmd_spotlight.c | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/source3/rpcclient/cmd_spotlight.c b/source3/rpcclient/cmd_spotlight.c index 24db9893df6..64fe321089c 100644 --- a/source3/rpcclient/cmd_spotlight.c +++ b/source3/rpcclient/cmd_spotlight.c @@ -144,13 +144,6 @@ static NTSTATUS cmd_mdssvc_fetch_properties( } request_blob.size = max_fragment_size; - response_blob.spotlight_blob = talloc_array(mem_ctx, uint8_t, max_fragment_size); - if (response_blob.spotlight_blob == NULL) { - status = NT_STATUS_INTERNAL_ERROR; - goto done; - } - response_blob.size = max_fragment_size; - len = sl_pack(d, (char *)request_blob.spotlight_blob, request_blob.size); if (len == -1) { status = NT_STATUS_INTERNAL_ERROR; @@ -368,15 +361,6 @@ static NTSTATUS cmd_mdssvc_fetch_attributes( } request_blob.size = max_fragment_size; - response_blob.spotlight_blob = talloc_array(mem_ctx, - uint8_t, - max_fragment_size); - if (response_blob.spotlight_blob == NULL) { - status = NT_STATUS_INTERNAL_ERROR; - goto done; - } - response_blob.size = max_fragment_size; - len = sl_pack(d, (char *)request_blob.spotlight_blob, request_blob.size); if (len == -1) { status = NT_STATUS_INTERNAL_ERROR; -- 2.34.1 From 34f9f1b37ec07a4f233fe90a0e97ce504e0cdffb Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 20 Jun 2023 11:42:10 +0200 Subject: [PATCH 15/25] CVE-2023-34968: mdssvc: remove response blob allocation This is alreay done by NDR for us. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15388 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher --- source3/rpc_server/mdssvc/srv_mdssvc_nt.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/source3/rpc_server/mdssvc/srv_mdssvc_nt.c b/source3/rpc_server/mdssvc/srv_mdssvc_nt.c index 2fca15cb8a8..2fec2bb6725 100644 --- a/source3/rpc_server/mdssvc/srv_mdssvc_nt.c +++ b/source3/rpc_server/mdssvc/srv_mdssvc_nt.c @@ -164,7 +164,6 @@ void _mdssvc_cmd(struct pipes_struct *p, struct mdssvc_cmd *r) struct auth_session_info *session_info = dcesrv_call_session_info(dce_call); bool ok; - char *rbuf; struct mds_ctx *mds_ctx; NTSTATUS status; @@ -221,14 +220,6 @@ void _mdssvc_cmd(struct pipes_struct *p, struct mdssvc_cmd *r) return; } - rbuf = talloc_zero_array(p->mem_ctx, char, r->in.max_fragment_size1); - if (rbuf == NULL) { - p->fault_state = DCERPC_FAULT_CANT_PERFORM; - return; - } - r->out.response_blob->spotlight_blob = (uint8_t *)rbuf; - r->out.response_blob->size = r->in.max_fragment_size1; - /* We currently don't use fragmentation at the mdssvc RPC layer */ *r->out.fragment = 0; -- 2.34.1 From 0fdfc85f28a21f66aa2f1d7e337fe9184368e972 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 20 Jun 2023 11:05:22 +0200 Subject: [PATCH 16/25] CVE-2023-34968: mdssvc: switch to doing an early return Just reduce indentation of the code handling the success case. No change in behaviour. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15388 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher --- source3/rpc_server/mdssvc/mdssvc.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/source3/rpc_server/mdssvc/mdssvc.c b/source3/rpc_server/mdssvc/mdssvc.c index 72936a99289..92afa9e5fd4 100644 --- a/source3/rpc_server/mdssvc/mdssvc.c +++ b/source3/rpc_server/mdssvc/mdssvc.c @@ -1817,19 +1817,21 @@ bool mds_dispatch(struct mds_ctx *mds_ctx, } ok = slcmd->function(mds_ctx, query, reply); - if (ok) { - DBG_DEBUG("%s", dalloc_dump(reply, 0)); - - len = sl_pack(reply, - (char *)response_blob->spotlight_blob, - response_blob->size); - if (len == -1) { - DBG_ERR("error packing Spotlight RPC reply\n"); - ok = false; - goto cleanup; - } - response_blob->length = len; + if (!ok) { + goto cleanup; + } + + DBG_DEBUG("%s", dalloc_dump(reply, 0)); + + len = sl_pack(reply, + (char *)response_blob->spotlight_blob, + response_blob->size); + if (len == -1) { + DBG_ERR("error packing Spotlight RPC reply\n"); + ok = false; + goto cleanup; } + response_blob->length = len; cleanup: talloc_free(query); -- 2.34.1 From d6b9c5234ffc6bf415156c693aac3256d17e259c Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 19 Jun 2023 18:16:57 +0200 Subject: [PATCH 17/25] CVE-2023-34968: mdssvc: introduce an allocating wrapper to sl_pack() sl_pack_alloc() does the buffer allocation that previously all callers of sl_pack() did themselves. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15388 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher --- source3/rpc_client/cli_mdssvc_util.c | 80 +++++------------------ source3/rpc_server/mdssvc/marshalling.c | 35 ++++++++-- source3/rpc_server/mdssvc/marshalling.h | 9 ++- source3/rpc_server/mdssvc/mdssvc.c | 18 ++--- source3/rpc_server/mdssvc/mdssvc.h | 5 +- source3/rpc_server/mdssvc/srv_mdssvc_nt.c | 5 +- source3/rpcclient/cmd_spotlight.c | 32 ++------- source4/torture/rpc/mdssvc.c | 24 ++----- 8 files changed, 80 insertions(+), 128 deletions(-) diff --git a/source3/rpc_client/cli_mdssvc_util.c b/source3/rpc_client/cli_mdssvc_util.c index 892a844e71a..a39202d0c99 100644 --- a/source3/rpc_client/cli_mdssvc_util.c +++ b/source3/rpc_client/cli_mdssvc_util.c @@ -42,7 +42,7 @@ NTSTATUS mdscli_blob_search(TALLOC_CTX *mem_ctx, sl_array_t *scope_array = NULL; double dval; uint64_t uint64val; - ssize_t len; + NTSTATUS status; int ret; d = dalloc_new(mem_ctx); @@ -209,23 +209,11 @@ NTSTATUS mdscli_blob_search(TALLOC_CTX *mem_ctx, return NT_STATUS_NO_MEMORY; } - blob->spotlight_blob = talloc_array(mem_ctx, - uint8_t, - ctx->max_fragment_size); - if (blob->spotlight_blob == NULL) { - TALLOC_FREE(d); - return NT_STATUS_NO_MEMORY; - } - blob->size = ctx->max_fragment_size; - - len = sl_pack(d, (char *)blob->spotlight_blob, blob->size); + status = sl_pack_alloc(mem_ctx, d, blob, ctx->max_fragment_size); TALLOC_FREE(d); - if (len == -1) { - return NT_STATUS_NO_MEMORY; + if (!NT_STATUS_IS_OK(status)) { + return status; } - - blob->length = len; - blob->size = len; return NT_STATUS_OK; } @@ -238,7 +226,7 @@ NTSTATUS mdscli_blob_get_results(TALLOC_CTX *mem_ctx, uint64_t *uint64p = NULL; sl_array_t *array = NULL; sl_array_t *cmd_array = NULL; - ssize_t len; + NTSTATUS status; int ret; d = dalloc_new(mem_ctx); @@ -293,23 +281,11 @@ NTSTATUS mdscli_blob_get_results(TALLOC_CTX *mem_ctx, return NT_STATUS_NO_MEMORY; } - blob->spotlight_blob = talloc_array(mem_ctx, - uint8_t, - ctx->max_fragment_size); - if (blob->spotlight_blob == NULL) { - TALLOC_FREE(d); - return NT_STATUS_NO_MEMORY; - } - blob->size = ctx->max_fragment_size; - - len = sl_pack(d, (char *)blob->spotlight_blob, blob->size); + status = sl_pack_alloc(mem_ctx, d, blob, ctx->max_fragment_size); TALLOC_FREE(d); - if (len == -1) { - return NT_STATUS_NO_MEMORY; + if (!NT_STATUS_IS_OK(status)) { + return status; } - - blob->length = len; - blob->size = len; return NT_STATUS_OK; } @@ -325,7 +301,7 @@ NTSTATUS mdscli_blob_get_path(TALLOC_CTX *mem_ctx, sl_array_t *cmd_array = NULL; sl_array_t *attr_array = NULL; sl_cnids_t *cnids = NULL; - ssize_t len; + NTSTATUS status; int ret; d = dalloc_new(mem_ctx); @@ -426,23 +402,11 @@ NTSTATUS mdscli_blob_get_path(TALLOC_CTX *mem_ctx, return NT_STATUS_NO_MEMORY; } - blob->spotlight_blob = talloc_array(mem_ctx, - uint8_t, - ctx->max_fragment_size); - if (blob->spotlight_blob == NULL) { - TALLOC_FREE(d); - return NT_STATUS_NO_MEMORY; - } - blob->size = ctx->max_fragment_size; - - len = sl_pack(d, (char *)blob->spotlight_blob, blob->size); + status = sl_pack_alloc(mem_ctx, d, blob, ctx->max_fragment_size); TALLOC_FREE(d); - if (len == -1) { - return NT_STATUS_NO_MEMORY; + if (!NT_STATUS_IS_OK(status)) { + return status; } - - blob->length = len; - blob->size = len; return NT_STATUS_OK; } @@ -455,7 +419,7 @@ NTSTATUS mdscli_blob_close_search(TALLOC_CTX *mem_ctx, uint64_t *uint64p = NULL; sl_array_t *array = NULL; sl_array_t *cmd_array = NULL; - ssize_t len; + NTSTATUS status; int ret; d = dalloc_new(mem_ctx); @@ -510,22 +474,10 @@ NTSTATUS mdscli_blob_close_search(TALLOC_CTX *mem_ctx, return NT_STATUS_NO_MEMORY; } - blob->spotlight_blob = talloc_array(mem_ctx, - uint8_t, - ctx->max_fragment_size); - if (blob->spotlight_blob == NULL) { - TALLOC_FREE(d); - return NT_STATUS_NO_MEMORY; - } - blob->size = ctx->max_fragment_size; - - len = sl_pack(d, (char *)blob->spotlight_blob, blob->size); + status = sl_pack_alloc(mem_ctx, d, blob, ctx->max_fragment_size); TALLOC_FREE(d); - if (len == -1) { - return NT_STATUS_NO_MEMORY; + if (!NT_STATUS_IS_OK(status)) { + return status; } - - blob->length = len; - blob->size = len; return NT_STATUS_OK; } diff --git a/source3/rpc_server/mdssvc/marshalling.c b/source3/rpc_server/mdssvc/marshalling.c index 441d41160f1..34bfda5eca6 100644 --- a/source3/rpc_server/mdssvc/marshalling.c +++ b/source3/rpc_server/mdssvc/marshalling.c @@ -78,6 +78,7 @@ static ssize_t sl_unpack_loop(DALLOC_CTX *query, const char *buf, ssize_t offset, size_t bufsize, int count, ssize_t toc_offset, int encoding); +static ssize_t sl_pack(DALLOC_CTX *query, char *buf, size_t bufsize); /****************************************************************************** * Wrapper functions for the *VAL macros with bound checking @@ -1190,11 +1191,7 @@ static ssize_t sl_unpack_loop(DALLOC_CTX *query, return offset; } -/****************************************************************************** - * Global functions for packing und unpacking - ******************************************************************************/ - -ssize_t sl_pack(DALLOC_CTX *query, char *buf, size_t bufsize) +static ssize_t sl_pack(DALLOC_CTX *query, char *buf, size_t bufsize) { ssize_t result; char *toc_buf; @@ -1274,6 +1271,34 @@ ssize_t sl_pack(DALLOC_CTX *query, char *buf, size_t bufsize) return len; } +/****************************************************************************** + * Global functions for packing und unpacking + ******************************************************************************/ + +NTSTATUS sl_pack_alloc(TALLOC_CTX *mem_ctx, + DALLOC_CTX *d, + struct mdssvc_blob *b, + size_t max_fragment_size) +{ + ssize_t len; + + b->spotlight_blob = talloc_zero_array(mem_ctx, + uint8_t, + max_fragment_size); + if (b->spotlight_blob == NULL) { + return NT_STATUS_NO_MEMORY; + } + + len = sl_pack(d, (char *)b->spotlight_blob, max_fragment_size); + if (len == -1) { + return NT_STATUS_DATA_ERROR; + } + + b->length = len; + b->size = len; + return NT_STATUS_OK; +} + bool sl_unpack(DALLOC_CTX *query, const char *buf, size_t bufsize) { ssize_t result; diff --git a/source3/rpc_server/mdssvc/marshalling.h b/source3/rpc_server/mdssvc/marshalling.h index 086ca740604..2cc1b44712c 100644 --- a/source3/rpc_server/mdssvc/marshalling.h +++ b/source3/rpc_server/mdssvc/marshalling.h @@ -22,6 +22,9 @@ #define _MDSSVC_MARSHALLING_H #include "dalloc.h" +#include "libcli/util/ntstatus.h" +#include "lib/util/data_blob.h" +#include "librpc/gen_ndr/mdssvc.h" #define MAX_SL_FRAGMENT_SIZE 0xFFFFF @@ -49,7 +52,11 @@ typedef struct { * Function declarations ******************************************************************************/ -extern ssize_t sl_pack(DALLOC_CTX *query, char *buf, size_t bufsize); +extern NTSTATUS sl_pack_alloc(TALLOC_CTX *mem_ctx, + DALLOC_CTX *d, + struct mdssvc_blob *b, + size_t max_fragment_size); + extern bool sl_unpack(DALLOC_CTX *query, const char *buf, size_t bufsize); #endif diff --git a/source3/rpc_server/mdssvc/mdssvc.c b/source3/rpc_server/mdssvc/mdssvc.c index 92afa9e5fd4..0e62cba7038 100644 --- a/source3/rpc_server/mdssvc/mdssvc.c +++ b/source3/rpc_server/mdssvc/mdssvc.c @@ -1745,11 +1745,11 @@ error: **/ bool mds_dispatch(struct mds_ctx *mds_ctx, struct mdssvc_blob *request_blob, - struct mdssvc_blob *response_blob) + struct mdssvc_blob *response_blob, + size_t max_fragment_size) { bool ok; int ret; - ssize_t len; DALLOC_CTX *query = NULL; DALLOC_CTX *reply = NULL; char *rpccmd; @@ -1757,6 +1757,7 @@ bool mds_dispatch(struct mds_ctx *mds_ctx, const struct smb_filename conn_basedir = { .base_name = mds_ctx->conn->connectpath, }; + NTSTATUS status; if (CHECK_DEBUGLVL(10)) { const struct sl_query *slq; @@ -1823,15 +1824,14 @@ bool mds_dispatch(struct mds_ctx *mds_ctx, DBG_DEBUG("%s", dalloc_dump(reply, 0)); - len = sl_pack(reply, - (char *)response_blob->spotlight_blob, - response_blob->size); - if (len == -1) { - DBG_ERR("error packing Spotlight RPC reply\n"); - ok = false; + status = sl_pack_alloc(response_blob, + reply, + response_blob, + max_fragment_size); + if (!NT_STATUS_IS_OK(status)) { + DBG_ERR("sl_pack_alloc() failed\n"); goto cleanup; } - response_blob->length = len; cleanup: talloc_free(query); diff --git a/source3/rpc_server/mdssvc/mdssvc.h b/source3/rpc_server/mdssvc/mdssvc.h index ff36b329f2b..e1fc0709ab1 100644 --- a/source3/rpc_server/mdssvc/mdssvc.h +++ b/source3/rpc_server/mdssvc/mdssvc.h @@ -158,9 +158,10 @@ NTSTATUS mds_init_ctx(TALLOC_CTX *mem_ctx, const char *sharename, const char *path, struct mds_ctx **_mds_ctx); -extern bool mds_dispatch(struct mds_ctx *query_ctx, +extern bool mds_dispatch(struct mds_ctx *mds_ctx, struct mdssvc_blob *request_blob, - struct mdssvc_blob *response_blob); + struct mdssvc_blob *response_blob, + size_t max_fragment_size); bool mds_add_result(struct sl_query *slq, const char *path); #endif /* _MDSSVC_H */ diff --git a/source3/rpc_server/mdssvc/srv_mdssvc_nt.c b/source3/rpc_server/mdssvc/srv_mdssvc_nt.c index 2fec2bb6725..ae6a73605e1 100644 --- a/source3/rpc_server/mdssvc/srv_mdssvc_nt.c +++ b/source3/rpc_server/mdssvc/srv_mdssvc_nt.c @@ -223,7 +223,10 @@ void _mdssvc_cmd(struct pipes_struct *p, struct mdssvc_cmd *r) /* We currently don't use fragmentation at the mdssvc RPC layer */ *r->out.fragment = 0; - ok = mds_dispatch(mds_ctx, &r->in.request_blob, r->out.response_blob); + ok = mds_dispatch(mds_ctx, + &r->in.request_blob, + r->out.response_blob, + r->in.max_fragment_size1); if (ok) { *r->out.unkn9 = 0; } else { diff --git a/source3/rpcclient/cmd_spotlight.c b/source3/rpcclient/cmd_spotlight.c index 64fe321089c..ba3f61fd4b0 100644 --- a/source3/rpcclient/cmd_spotlight.c +++ b/source3/rpcclient/cmd_spotlight.c @@ -43,7 +43,6 @@ static NTSTATUS cmd_mdssvc_fetch_properties( uint32_t unkn3; /* server always returns 0 ? */ struct mdssvc_blob request_blob; struct mdssvc_blob response_blob; - ssize_t len; uint32_t max_fragment_size = 64 * 1024; DALLOC_CTX *d, *mds_reply; uint64_t *uint64var; @@ -137,20 +136,10 @@ static NTSTATUS cmd_mdssvc_fetch_properties( goto done; } - request_blob.spotlight_blob = talloc_array(mem_ctx, uint8_t, max_fragment_size); - if (request_blob.spotlight_blob == NULL) { - status = NT_STATUS_INTERNAL_ERROR; - goto done; - } - request_blob.size = max_fragment_size; - - len = sl_pack(d, (char *)request_blob.spotlight_blob, request_blob.size); - if (len == -1) { - status = NT_STATUS_INTERNAL_ERROR; + status = sl_pack_alloc(mem_ctx, d, &request_blob, max_fragment_size); + if (!NT_STATUS_IS_OK(status)) { goto done; } - request_blob.length = len; - request_blob.size = len; status = dcerpc_mdssvc_cmd(b, mem_ctx, &share_handle, @@ -204,7 +193,6 @@ static NTSTATUS cmd_mdssvc_fetch_attributes( uint32_t unkn3; /* server always returns 0 ? */ struct mdssvc_blob request_blob; struct mdssvc_blob response_blob; - ssize_t len; uint32_t max_fragment_size = 64 * 1024; DALLOC_CTX *d, *mds_reply; uint64_t *uint64var; @@ -352,22 +340,10 @@ static NTSTATUS cmd_mdssvc_fetch_attributes( goto done; } - request_blob.spotlight_blob = talloc_array(mem_ctx, - uint8_t, - max_fragment_size); - if (request_blob.spotlight_blob == NULL) { - status = NT_STATUS_INTERNAL_ERROR; - goto done; - } - request_blob.size = max_fragment_size; - - len = sl_pack(d, (char *)request_blob.spotlight_blob, request_blob.size); - if (len == -1) { - status = NT_STATUS_INTERNAL_ERROR; + status = sl_pack_alloc(mem_ctx, d, &request_blob, max_fragment_size); + if (!NT_STATUS_IS_OK(status)) { goto done; } - request_blob.length = len; - request_blob.size = len; status = dcerpc_mdssvc_cmd(b, mem_ctx, &share_handle, diff --git a/source4/torture/rpc/mdssvc.c b/source4/torture/rpc/mdssvc.c index a16bd5b47e3..afe7068ee1b 100644 --- a/source4/torture/rpc/mdssvc.c +++ b/source4/torture/rpc/mdssvc.c @@ -744,11 +744,9 @@ static bool test_sl_dict_type_safety(struct torture_context *tctx, 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"); + status = sl_pack_alloc(tctx, d, &request_blob, 64 * 1024); + torture_assert_ntstatus_ok_goto(tctx, status, ok, done, + "sl_pack_alloc() failed\n"); status = dcerpc_mdssvc_cmd(b, state, @@ -835,7 +833,6 @@ static bool test_mdssvc_fetch_attr_unknown_cnid(struct torture_context *tctx, const char *path_type = NULL; uint64_t ino64; NTSTATUS status; - ssize_t len; int ret; bool ok = true; @@ -900,18 +897,9 @@ static bool test_mdssvc_fetch_attr_unknown_cnid(struct torture_context *tctx, ret = dalloc_add(array, cnids, sl_cnids_t); torture_assert_goto(tctx, ret == 0, ret, done, "dalloc_add failed\n"); - request_blob.spotlight_blob = talloc_array(state, - uint8_t, - max_fragment_size); - torture_assert_not_null_goto(tctx, request_blob.spotlight_blob, - ret, done, "dalloc_zero failed\n"); - request_blob.size = max_fragment_size; - - len = sl_pack(d, (char *)request_blob.spotlight_blob, request_blob.size); - torture_assert_goto(tctx, len != -1, ret, done, "sl_pack failed\n"); - - request_blob.length = len; - request_blob.size = len; + status = sl_pack_alloc(tctx, d, &request_blob, max_fragment_size); + torture_assert_ntstatus_ok_goto(tctx, status, ok, done, + "sl_pack_alloc() failed\n"); status = dcerpc_mdssvc_cmd(b, state, -- 2.34.1 From cecd415a0abd857831a33d1f0719b870d85e7966 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 17 Jun 2023 13:53:27 +0200 Subject: [PATCH 18/25] CVE-2023-34968: mdscli: return share relative paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The next commit will change the Samba Spotlight server to return absolute paths that start with the sharename as "/SHARENAME/..." followed by the share path relative appended. So given a share [spotlight] path = /foo/bar spotlight = yes and a file inside this share with a full path of /foo/bar/dir/file previously a search that matched this file would returns the absolute server-side pato of the file, ie /foo/bar/dir/file This will be change to /spotlight/dir/file As currently the mdscli library and hence the mdsearch tool print out these paths returned from the server, we have to change the output to accomodate these fake paths. The only way to do this sensibly is by makeing the paths relative to the containing share, so just dir/file in the example above. The client learns about the share root path prefix – real server-side of fake in the future – in an initial handshake in the "share_path" out argument of the mdssvc_open() RPC call, so the client can use this path to convert the absolute path to relative. There is however an additional twist: the macOS Spotlight server prefixes this absolute path with another prefix, typically "/System/Volumes/Data", so in the example above the full path for the same search would be /System/Volumes/Data/foo/bar/dir/file So macOS does return the full server-side path too, just prefixed with an additional path. This path prefixed can be queried by the client in the mdssvc_cmd() RPC call with an Spotlight command of "fetchPropertiesForContext:" and the path is returned in a dictionary with key "kMDSStorePathScopes". Samba just returns "/" for this. Currently the mdscli library doesn't issue this Spotlight RPC request (fetchPropertiesForContext), so this is added in this commit. In the end, all search result paths are stripped of the combined prefix kMDSStorePathScopes + share_path (from mdssvc_open). eg kMDSStorePathScopes = /System/Volumes/Data share_path = /foo/bar search result = /System/Volumes/Data/foo/bar/dir/file relative path returned by mdscli = dir/file Makes sense? :) BUG: https://bugzilla.samba.org/show_bug.cgi?id=15388 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher --- python/samba/tests/blackbox/mdsearch.py | 8 +- python/samba/tests/dcerpc/mdssvc.py | 26 ++-- source3/rpc_client/cli_mdssvc.c | 155 +++++++++++++++++++++++- source3/rpc_client/cli_mdssvc_private.h | 4 + source3/rpc_client/cli_mdssvc_util.c | 68 +++++++++++ source3/rpc_client/cli_mdssvc_util.h | 4 + 6 files changed, 245 insertions(+), 20 deletions(-) diff --git a/python/samba/tests/blackbox/mdsearch.py b/python/samba/tests/blackbox/mdsearch.py index c9156ae6e0e..c8e75661f15 100644 --- a/python/samba/tests/blackbox/mdsearch.py +++ b/python/samba/tests/blackbox/mdsearch.py @@ -76,10 +76,7 @@ class MdfindBlackboxTests(BlackboxTestCase): self.t.start() time.sleep(1) - pipe = mdssvc.mdssvc('ncacn_np:fileserver[/pipe/mdssvc]', self.get_loadparm()) - conn = mdscli.conn(pipe, 'spotlight', '/foo') - self.sharepath = conn.sharepath() - conn.disconnect(pipe) + self.sharepath = os.environ["LOCAL_PATH"] for file in testfiles: f = open("%s/%s" % (self.sharepath, file), "w") @@ -126,5 +123,4 @@ class MdfindBlackboxTests(BlackboxTestCase): output = self.check_output("mdsearch --configfile=%s -U %s%%%s fileserver spotlight '*==\"samba*\"'" % (config, username, password)) actual = output.decode('utf-8').splitlines() - expected = ["%s/%s" % (self.sharepath, file) for file in testfiles] - self.assertEqual(expected, actual) + self.assertEqual(testfiles, actual) diff --git a/python/samba/tests/dcerpc/mdssvc.py b/python/samba/tests/dcerpc/mdssvc.py index b0df509ddc7..5002e5d26d6 100644 --- a/python/samba/tests/dcerpc/mdssvc.py +++ b/python/samba/tests/dcerpc/mdssvc.py @@ -84,10 +84,11 @@ class MdssvcTests(RpcInterfaceTestCase): self.t = threading.Thread(target=MdssvcTests.http_server, args=(self,)) self.t.setDaemon(True) self.t.start() + self.sharepath = os.environ["LOCAL_PATH"] time.sleep(1) conn = mdscli.conn(self.pipe, 'spotlight', '/foo') - self.sharepath = conn.sharepath() + self.fakepath = conn.sharepath() conn.disconnect(self.pipe) for file in testfiles: @@ -105,12 +106,11 @@ class MdssvcTests(RpcInterfaceTestCase): self.server.serve_forever() def run_test(self, query, expect, json_in, json_out): - expect = [s.replace("%BASEPATH%", self.sharepath) for s in expect] self.server.json_in = json_in.replace("%BASEPATH%", self.sharepath) self.server.json_out = json_out.replace("%BASEPATH%", self.sharepath) self.conn = mdscli.conn(self.pipe, 'spotlight', '/foo') - search = self.conn.search(self.pipe, query, self.sharepath) + search = self.conn.search(self.pipe, query, self.fakepath) # Give it some time, the get_results() below returns immediately # what's available, so if we ask to soon, we might get back no results @@ -141,7 +141,7 @@ class MdssvcTests(RpcInterfaceTestCase): ] } }''' - exp_results = ["%BASEPATH%/foo", "%BASEPATH%/bar"] + exp_results = ["foo", "bar"] self.run_test('*=="samba*"', exp_results, exp_json_query, fake_json_response) def test_mdscli_search_escapes(self): @@ -181,14 +181,14 @@ class MdssvcTests(RpcInterfaceTestCase): } }''' exp_results = [ - r"%BASEPATH%/x+x", - r"%BASEPATH%/x*x", - r"%BASEPATH%/x=x", - r"%BASEPATH%/x'x", - r"%BASEPATH%/x?x", - r"%BASEPATH%/x x", - r"%BASEPATH%/x(x", - "%BASEPATH%/x\"x", - r"%BASEPATH%/x\x", + r"x+x", + r"x*x", + r"x=x", + r"x'x", + r"x?x", + r"x x", + r"x(x", + "x\"x", + r"x\x", ] self.run_test(sl_query, exp_results, exp_json_query, fake_json_response) diff --git a/source3/rpc_client/cli_mdssvc.c b/source3/rpc_client/cli_mdssvc.c index 07c19b51dd4..03aed61c00c 100644 --- a/source3/rpc_client/cli_mdssvc.c +++ b/source3/rpc_client/cli_mdssvc.c @@ -43,10 +43,12 @@ char *mdscli_get_basepath(TALLOC_CTX *mem_ctx, struct mdscli_connect_state { struct tevent_context *ev; struct mdscli_ctx *mdscli_ctx; + struct mdssvc_blob response_blob; }; static void mdscli_connect_open_done(struct tevent_req *subreq); static void mdscli_connect_unknown1_done(struct tevent_req *subreq); +static void mdscli_connect_fetch_props_done(struct tevent_req *subreq); struct tevent_req *mdscli_connect_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, @@ -111,6 +113,7 @@ static void mdscli_connect_open_done(struct tevent_req *subreq) struct mdscli_connect_state *state = tevent_req_data( req, struct mdscli_connect_state); struct mdscli_ctx *mdscli_ctx = state->mdscli_ctx; + size_t share_path_len; NTSTATUS status; status = dcerpc_mdssvc_open_recv(subreq, state); @@ -120,6 +123,18 @@ static void mdscli_connect_open_done(struct tevent_req *subreq) return; } + share_path_len = strlen(mdscli_ctx->mdscmd_open.share_path); + if (share_path_len < 1 || share_path_len > UINT16_MAX) { + tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR); + return; + } + mdscli_ctx->mdscmd_open.share_path_len = share_path_len; + + if (mdscli_ctx->mdscmd_open.share_path[share_path_len-1] == '/') { + mdscli_ctx->mdscmd_open.share_path[share_path_len-1] = '\0'; + mdscli_ctx->mdscmd_open.share_path_len--; + } + subreq = dcerpc_mdssvc_unknown1_send( state, state->ev, @@ -146,6 +161,8 @@ static void mdscli_connect_unknown1_done(struct tevent_req *subreq) subreq, struct tevent_req); struct mdscli_connect_state *state = tevent_req_data( req, struct mdscli_connect_state); + struct mdscli_ctx *mdscli_ctx = state->mdscli_ctx; + struct mdssvc_blob request_blob; NTSTATUS status; status = dcerpc_mdssvc_unknown1_recv(subreq, state); @@ -154,6 +171,108 @@ static void mdscli_connect_unknown1_done(struct tevent_req *subreq) return; } + status = mdscli_blob_fetch_props(state, + state->mdscli_ctx, + &request_blob); + if (tevent_req_nterror(req, status)) { + return; + } + + subreq = dcerpc_mdssvc_cmd_send(state, + state->ev, + mdscli_ctx->bh, + &mdscli_ctx->ph, + 0, + mdscli_ctx->dev, + mdscli_ctx->mdscmd_open.unkn2, + 0, + mdscli_ctx->flags, + request_blob, + 0, + mdscli_ctx->max_fragment_size, + 1, + mdscli_ctx->max_fragment_size, + 0, + 0, + &mdscli_ctx->mdscmd_cmd.fragment, + &state->response_blob, + &mdscli_ctx->mdscmd_cmd.unkn9); + if (tevent_req_nomem(subreq, req)) { + return; + } + tevent_req_set_callback(subreq, mdscli_connect_fetch_props_done, req); + mdscli_ctx->async_pending++; + return; +} + +static void mdscli_connect_fetch_props_done(struct tevent_req *subreq) +{ + struct tevent_req *req = tevent_req_callback_data( + subreq, struct tevent_req); + struct mdscli_connect_state *state = tevent_req_data( + req, struct mdscli_connect_state); + struct mdscli_ctx *mdscli_ctx = state->mdscli_ctx; + DALLOC_CTX *d = NULL; + sl_array_t *path_scope_array = NULL; + char *path_scope = NULL; + NTSTATUS status; + bool ok; + + status = dcerpc_mdssvc_cmd_recv(subreq, state); + TALLOC_FREE(subreq); + state->mdscli_ctx->async_pending--; + if (tevent_req_nterror(req, status)) { + return; + } + + d = dalloc_new(state); + if (tevent_req_nomem(d, req)) { + return; + } + + ok = sl_unpack(d, + (char *)state->response_blob.spotlight_blob, + state->response_blob.length); + if (!ok) { + tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR); + return; + } + + path_scope_array = dalloc_value_for_key(d, + "DALLOC_CTX", 0, + "kMDSStorePathScopes", + "sl_array_t"); + if (path_scope_array == NULL) { + DBG_ERR("Missing kMDSStorePathScopes\n"); + tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR); + return; + } + + path_scope = dalloc_get(path_scope_array, "char *", 0); + if (path_scope == NULL) { + DBG_ERR("Missing path in kMDSStorePathScopes\n"); + tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR); + return; + } + + mdscli_ctx->path_scope_len = strlen(path_scope); + if (mdscli_ctx->path_scope_len < 1 || + mdscli_ctx->path_scope_len > UINT16_MAX) + { + DBG_ERR("Bad path_scope: %s\n", path_scope); + tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR); + return; + } + mdscli_ctx->path_scope = talloc_strdup(mdscli_ctx, path_scope); + if (tevent_req_nomem(mdscli_ctx->path_scope, req)) { + return; + } + + if (mdscli_ctx->path_scope[mdscli_ctx->path_scope_len-1] == '/') { + mdscli_ctx->path_scope[mdscli_ctx->path_scope_len-1] = '\0'; + mdscli_ctx->path_scope_len--; + } + tevent_req_done(req); } @@ -697,7 +816,10 @@ static void mdscli_get_path_done(struct tevent_req *subreq) struct mdscli_get_path_state *state = tevent_req_data( req, struct mdscli_get_path_state); DALLOC_CTX *d = NULL; + size_t pathlen; + size_t prefixlen; char *path = NULL; + const char *p = NULL; NTSTATUS status; bool ok; @@ -732,7 +854,38 @@ static void mdscli_get_path_done(struct tevent_req *subreq) tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR); return; } - state->path = talloc_move(state, &path); + + /* Path is prefixed by /PATHSCOPE/SHARENAME/, strip it */ + pathlen = strlen(path); + + /* + * path_scope_len and share_path_len are already checked to be smaller + * then UINT16_MAX so this can't overflow + */ + prefixlen = state->mdscli_ctx->path_scope_len + + state->mdscli_ctx->mdscmd_open.share_path_len; + + if (pathlen < prefixlen) { + DBG_DEBUG("Bad path: %s\n", path); + tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); + return; + } + + p = path + prefixlen; + while (*p == '/') { + p++; + } + if (*p == '\0') { + DBG_DEBUG("Bad path: %s\n", path); + tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); + return; + } + + state->path = talloc_strdup(state, p); + if (state->path == NULL) { + tevent_req_nterror(req, NT_STATUS_NO_MEMORY); + return; + } DBG_DEBUG("path: %s\n", state->path); tevent_req_done(req); diff --git a/source3/rpc_client/cli_mdssvc_private.h b/source3/rpc_client/cli_mdssvc_private.h index 031af85bf58..77f300c09cc 100644 --- a/source3/rpc_client/cli_mdssvc_private.h +++ b/source3/rpc_client/cli_mdssvc_private.h @@ -42,6 +42,7 @@ struct mdscli_ctx { /* cmd specific or unknown fields */ struct { char share_path[1025]; + size_t share_path_len; uint32_t unkn2; uint32_t unkn3; } mdscmd_open; @@ -56,6 +57,9 @@ struct mdscli_ctx { struct { uint32_t status; } mdscmd_close; + + char *path_scope; + size_t path_scope_len; }; struct mdscli_search_ctx { diff --git a/source3/rpc_client/cli_mdssvc_util.c b/source3/rpc_client/cli_mdssvc_util.c index a39202d0c99..1eaaca715a8 100644 --- a/source3/rpc_client/cli_mdssvc_util.c +++ b/source3/rpc_client/cli_mdssvc_util.c @@ -28,6 +28,74 @@ #include "rpc_server/mdssvc/dalloc.h" #include "rpc_server/mdssvc/marshalling.h" +NTSTATUS mdscli_blob_fetch_props(TALLOC_CTX *mem_ctx, + struct mdscli_ctx *ctx, + struct mdssvc_blob *blob) +{ + DALLOC_CTX *d = NULL; + uint64_t *uint64p = NULL; + sl_array_t *array = NULL; + sl_array_t *cmd_array = NULL; + NTSTATUS status; + int ret; + + d = dalloc_new(mem_ctx); + if (d == NULL) { + return NT_STATUS_NO_MEMORY; + } + + array = dalloc_zero(d, sl_array_t); + if (array == NULL) { + TALLOC_FREE(d); + return NT_STATUS_NO_MEMORY; + } + + ret = dalloc_add(d, array, sl_array_t); + if (ret != 0) { + TALLOC_FREE(d); + return NT_STATUS_NO_MEMORY; + } + + cmd_array = dalloc_zero(d, sl_array_t); + if (cmd_array == NULL) { + TALLOC_FREE(d); + return NT_STATUS_NO_MEMORY; + } + + ret = dalloc_add(array, cmd_array, sl_array_t); + if (ret != 0) { + TALLOC_FREE(d); + return NT_STATUS_NO_MEMORY; + } + + ret = dalloc_stradd(cmd_array, "fetchPropertiesForContext:"); + if (ret != 0) { + TALLOC_FREE(d); + return NT_STATUS_NO_MEMORY; + } + + uint64p = talloc_zero_array(cmd_array, uint64_t, 2); + if (uint64p == NULL) { + TALLOC_FREE(d); + return NT_STATUS_NO_MEMORY; + } + + talloc_set_name(uint64p, "uint64_t *"); + + ret = dalloc_add(cmd_array, uint64p, uint64_t *); + if (ret != 0) { + TALLOC_FREE(d); + return NT_STATUS_NO_MEMORY; + } + + status = sl_pack_alloc(mem_ctx, d, blob, ctx->max_fragment_size); + TALLOC_FREE(d); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + return NT_STATUS_OK; +} + NTSTATUS mdscli_blob_search(TALLOC_CTX *mem_ctx, struct mdscli_search_ctx *search, struct mdssvc_blob *blob) diff --git a/source3/rpc_client/cli_mdssvc_util.h b/source3/rpc_client/cli_mdssvc_util.h index 7a98c854526..3f324758c70 100644 --- a/source3/rpc_client/cli_mdssvc_util.h +++ b/source3/rpc_client/cli_mdssvc_util.h @@ -21,6 +21,10 @@ #ifndef _MDSCLI_UTIL_H_ #define _MDSCLI_UTIL_H_ +NTSTATUS mdscli_blob_fetch_props(TALLOC_CTX *mem_ctx, + struct mdscli_ctx *ctx, + struct mdssvc_blob *blob); + NTSTATUS mdscli_blob_search(TALLOC_CTX *mem_ctx, struct mdscli_search_ctx *search, struct mdssvc_blob *blob); -- 2.34.1 From 1809843614bbd407db221ace2c90faf9259065b0 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 5 Jun 2023 18:02:20 +0200 Subject: [PATCH 19/25] CVE-2023-34968: mdssvc: return a fake share path Instead of returning the real server-side absolute path of shares and search results, return a fake absolute path replacing the path of the share with the share name, iow for a share "test" with a server-side path of "/foo/bar", we previously returned /foo/bar and /foo/bar/search/result and now return /test and /test/search/result BUG: https://bugzilla.samba.org/show_bug.cgi?id=15388 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher --- source3/rpc_server/mdssvc/mdssvc.c | 61 +++++++++++++++++++++-- source3/rpc_server/mdssvc/mdssvc.h | 1 + source3/rpc_server/mdssvc/srv_mdssvc_nt.c | 18 +++++-- 3 files changed, 73 insertions(+), 7 deletions(-) diff --git a/source3/rpc_server/mdssvc/mdssvc.c b/source3/rpc_server/mdssvc/mdssvc.c index 0e62cba7038..c616bdfa53b 100644 --- a/source3/rpc_server/mdssvc/mdssvc.c +++ b/source3/rpc_server/mdssvc/mdssvc.c @@ -520,11 +520,14 @@ static bool inode_map_add(struct sl_query *slq, bool mds_add_result(struct sl_query *slq, const char *path) { struct smb_filename *smb_fname = NULL; + const char *relative = NULL; + char *fake_path = NULL; struct stat_ex sb; uint32_t attr; uint64_t ino64; int result; NTSTATUS status; + bool sub; bool ok; /* @@ -611,6 +614,17 @@ bool mds_add_result(struct sl_query *slq, const char *path) } } + sub = subdir_of(slq->mds_ctx->spath, + slq->mds_ctx->spath_len, + path, + &relative); + if (!sub) { + DBG_ERR("[%s] is not inside [%s]\n", + path, slq->mds_ctx->spath); + slq->state = SLQ_STATE_ERROR; + return false; + } + /* * Add inode number and filemeta to result set, this is what * we return as part of the result set of a query @@ -623,18 +637,30 @@ bool mds_add_result(struct sl_query *slq, const char *path) slq->state = SLQ_STATE_ERROR; return false; } + + fake_path = talloc_asprintf(slq, + "/%s/%s", + slq->mds_ctx->sharename, + relative); + if (fake_path == NULL) { + slq->state = SLQ_STATE_ERROR; + return false; + } + ok = add_filemeta(slq->mds_ctx, slq->reqinfo, slq->query_results->fm_array, - path, + fake_path, &sb); if (!ok) { DBG_ERR("add_filemeta error\n"); + TALLOC_FREE(fake_path); slq->state = SLQ_STATE_ERROR; return false; } - ok = inode_map_add(slq, ino64, path, &sb); + ok = inode_map_add(slq, ino64, fake_path, &sb); + TALLOC_FREE(fake_path); if (!ok) { DEBUG(1, ("inode_map_add error\n")); slq->state = SLQ_STATE_ERROR; @@ -841,6 +867,32 @@ static void slq_close_timer(struct tevent_context *ev, } } +/** + * Translate a fake scope from the client like /sharename/dir + * to the real server-side path, replacing the "/sharename" part + * with the absolute server-side path of the share. + **/ +static bool mdssvc_real_scope(struct sl_query *slq, const char *fake_scope) +{ + size_t sname_len = strlen(slq->mds_ctx->sharename); + size_t fake_scope_len = strlen(fake_scope); + + if (fake_scope_len < sname_len + 1) { + DBG_ERR("Short scope [%s] for share [%s]\n", + fake_scope, slq->mds_ctx->sharename); + return false; + } + + slq->path_scope = talloc_asprintf(slq, + "%s%s", + slq->mds_ctx->spath, + fake_scope + sname_len + 1); + if (slq->path_scope == NULL) { + return false; + } + return true; +} + /** * Begin a search query **/ @@ -953,8 +1005,8 @@ static bool slrpc_open_query(struct mds_ctx *mds_ctx, goto error; } - slq->path_scope = talloc_strdup(slq, scope); - if (slq->path_scope == NULL) { + ok = mdssvc_real_scope(slq, scope); + if (!ok) { goto error; } @@ -1675,6 +1727,7 @@ NTSTATUS mds_init_ctx(TALLOC_CTX *mem_ctx, status = NT_STATUS_NO_MEMORY; goto error; } + mds_ctx->spath_len = strlen(path); mds_ctx->snum = snum; mds_ctx->pipe_session_info = session_info; diff --git a/source3/rpc_server/mdssvc/mdssvc.h b/source3/rpc_server/mdssvc/mdssvc.h index e1fc0709ab1..3b2ce250f1f 100644 --- a/source3/rpc_server/mdssvc/mdssvc.h +++ b/source3/rpc_server/mdssvc/mdssvc.h @@ -127,6 +127,7 @@ struct mds_ctx { int snum; const char *sharename; const char *spath; + size_t spath_len; struct connection_struct *conn; struct sl_query *query_list; /* list of active queries */ struct db_context *ino_path_map; /* dbwrap rbt for storing inode->path mappings */ diff --git a/source3/rpc_server/mdssvc/srv_mdssvc_nt.c b/source3/rpc_server/mdssvc/srv_mdssvc_nt.c index ae6a73605e1..c77e7185521 100644 --- a/source3/rpc_server/mdssvc/srv_mdssvc_nt.c +++ b/source3/rpc_server/mdssvc/srv_mdssvc_nt.c @@ -81,6 +81,7 @@ void _mdssvc_open(struct pipes_struct *p, struct mdssvc_open *r) loadparm_s3_global_substitution(); int snum; char *outpath = discard_const_p(char, r->out.share_path); + char *fake_path = NULL; char *path; NTSTATUS status; @@ -98,12 +99,21 @@ void _mdssvc_open(struct pipes_struct *p, struct mdssvc_open *r) path = lp_path(talloc_tos(), lp_sub, snum); if (path == NULL) { - DBG_ERR("Couldn't create policy handle for %s\n", + DBG_ERR("Couldn't create path for %s\n", r->in.share_name); p->fault_state = DCERPC_FAULT_CANT_PERFORM; return; } + fake_path = talloc_asprintf(p->mem_ctx, "/%s", r->in.share_name); + if (fake_path == NULL) { + DBG_ERR("Couldn't create fake share path for %s\n", + r->in.share_name); + talloc_free(path); + p->fault_state = DCERPC_FAULT_CANT_PERFORM; + return; + } + status = create_mdssvc_policy_handle(p->mem_ctx, p, snum, r->in.share_name, @@ -112,18 +122,20 @@ void _mdssvc_open(struct pipes_struct *p, struct mdssvc_open *r) if (NT_STATUS_EQUAL(status, NT_STATUS_WRONG_VOLUME)) { ZERO_STRUCTP(r->out.handle); talloc_free(path); + talloc_free(fake_path); return; } if (!NT_STATUS_IS_OK(status)) { DBG_ERR("Couldn't create policy handle for %s\n", r->in.share_name); talloc_free(path); + talloc_free(fake_path); p->fault_state = DCERPC_FAULT_CANT_PERFORM; return; } - strlcpy(outpath, path, 1024); - talloc_free(path); + strlcpy(outpath, fake_path, 1024); + talloc_free(fake_path); return; } -- 2.34.1 From 6e87858185f33318e6206ca831d46643172beb47 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Sat, 15 Jul 2023 17:20:32 +0200 Subject: [PATCH 20/25] netlogon.idl: add support for netr_LogonGetCapabilities response level 2 We don't have any documentation about this yet, but tests against a Windows Server 2022 patched with KB5028166 revealed that the response for query_level=2 is exactly the same as for querey_level=1. Until we know the reason for query_level=2 we won't use it as client nor support it in the server, but we want ndrdump to work. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15418 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 5f87888ed53320538cf773d64868390d8641a40e) --- librpc/idl/netlogon.idl | 1 + 1 file changed, 1 insertion(+) diff --git a/librpc/idl/netlogon.idl b/librpc/idl/netlogon.idl index d956a661fff..b51767136d3 100644 --- a/librpc/idl/netlogon.idl +++ b/librpc/idl/netlogon.idl @@ -1241,6 +1241,7 @@ interface netlogon /* Function 0x15 */ typedef [switch_type(uint32)] union { [case(1)] netr_NegotiateFlags server_capabilities; + [case(2)] netr_NegotiateFlags server_capabilities; } netr_Capabilities; NTSTATUS netr_LogonGetCapabilities( -- 2.34.1 From 5d48ea23decc42ba6f28bd8589ab27630cbe1670 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Sat, 15 Jul 2023 17:25:05 +0200 Subject: [PATCH 21/25] s4:torture/rpc: let rpc.schannel also check netr_LogonGetCapabilities with different levels The important change it that we expect DCERPC_NCA_S_FAULT_INVALID_TAG for unsupported query_levels, we allow it to work with servers with or without support for query_level=2. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15418 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 404ce08e9088968311c714e756f5d58ce2cef715) --- .../knownfail.d/netr_LogonGetCapabilities | 3 + source4/torture/rpc/netlogon.c | 77 ++++++++++++++++++- 2 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 selftest/knownfail.d/netr_LogonGetCapabilities diff --git a/selftest/knownfail.d/netr_LogonGetCapabilities b/selftest/knownfail.d/netr_LogonGetCapabilities new file mode 100644 index 00000000000..30aadf3bb9d --- /dev/null +++ b/selftest/knownfail.d/netr_LogonGetCapabilities @@ -0,0 +1,3 @@ +^samba3.rpc.schannel.*\.schannel\(nt4_dc +^samba3.rpc.schannel.*\.schannel\(ad_dc +^samba4.rpc.schannel.*\.schannel\(ad_dc diff --git a/source4/torture/rpc/netlogon.c b/source4/torture/rpc/netlogon.c index 2803dd13b46..8d5b7fad183 100644 --- a/source4/torture/rpc/netlogon.c +++ b/source4/torture/rpc/netlogon.c @@ -2056,8 +2056,47 @@ bool test_netlogon_capabilities(struct dcerpc_pipe *p, struct torture_context *t r.out.capabilities = &capabilities; r.out.return_authenticator = &return_auth; - torture_comment(tctx, "Testing LogonGetCapabilities\n"); + torture_comment(tctx, "Testing LogonGetCapabilities with query_level=0\n"); + r.in.query_level = 0; + ZERO_STRUCT(return_auth); + + /* + * we need to operate on a temporary copy of creds + * because dcerpc_netr_LogonGetCapabilities with + * an unknown query level returns DCERPC_NCA_S_FAULT_INVALID_TAG + * => NT_STATUS_RPC_ENUM_VALUE_OUT_OF_RANGE + * without looking a the authenticator. + */ + tmp_creds = *creds; + netlogon_creds_client_authenticator(&tmp_creds, &auth); + + status = dcerpc_netr_LogonGetCapabilities_r(b, tctx, &r); + torture_assert_ntstatus_equal(tctx, status, NT_STATUS_RPC_ENUM_VALUE_OUT_OF_RANGE, + "LogonGetCapabilities query_level=0 failed"); + + torture_comment(tctx, "Testing LogonGetCapabilities with query_level=3\n"); + + r.in.query_level = 3; + ZERO_STRUCT(return_auth); + + /* + * we need to operate on a temporary copy of creds + * because dcerpc_netr_LogonGetCapabilities with + * an unknown query level returns DCERPC_NCA_S_FAULT_INVALID_TAG + * => NT_STATUS_RPC_ENUM_VALUE_OUT_OF_RANGE + * without looking a the authenticator. + */ + tmp_creds = *creds; + netlogon_creds_client_authenticator(&tmp_creds, &auth); + + status = dcerpc_netr_LogonGetCapabilities_r(b, tctx, &r); + torture_assert_ntstatus_equal(tctx, status, NT_STATUS_RPC_ENUM_VALUE_OUT_OF_RANGE, + "LogonGetCapabilities query_level=0 failed"); + + torture_comment(tctx, "Testing LogonGetCapabilities with query_level=1\n"); + + r.in.query_level = 1; ZERO_STRUCT(return_auth); /* @@ -2077,6 +2116,42 @@ bool test_netlogon_capabilities(struct dcerpc_pipe *p, struct torture_context *t *creds = tmp_creds; + torture_assert(tctx, netlogon_creds_client_check(creds, + &r.out.return_authenticator->cred), + "Credential chaining failed"); + + torture_assert_int_equal(tctx, creds->negotiate_flags, + capabilities.server_capabilities, + "negotiate flags"); + + torture_comment(tctx, "Testing LogonGetCapabilities with query_level=2\n"); + + r.in.query_level = 2; + ZERO_STRUCT(return_auth); + + /* + * we need to operate on a temporary copy of creds + * because dcerpc_netr_LogonGetCapabilities with + * an query level 2 may returns DCERPC_NCA_S_FAULT_INVALID_TAG + * => NT_STATUS_RPC_ENUM_VALUE_OUT_OF_RANGE + * without looking a the authenticator. + */ + tmp_creds = *creds; + netlogon_creds_client_authenticator(&tmp_creds, &auth); + + status = dcerpc_netr_LogonGetCapabilities_r(b, tctx, &r); + if (NT_STATUS_EQUAL(status, NT_STATUS_RPC_ENUM_VALUE_OUT_OF_RANGE)) { + /* + * an server without KB5028166 returns + * DCERPC_NCA_S_FAULT_INVALID_TAG => + * NT_STATUS_RPC_ENUM_VALUE_OUT_OF_RANGE + */ + return true; + } + torture_assert_ntstatus_ok(tctx, status, "LogonGetCapabilities query_level=2 failed"); + + *creds = tmp_creds; + torture_assert(tctx, netlogon_creds_client_check(creds, &r.out.return_authenticator->cred), "Credential chaining failed"); -- 2.34.1 From 585df7e0220b57c022b0a7218e52ec811ee5b05e Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Sat, 15 Jul 2023 16:11:48 +0200 Subject: [PATCH 22/25] s4:rpc_server:netlogon: generate FAULT_INVALID_TAG for invalid netr_LogonGetCapabilities levels This is important as Windows clients with KB5028166 seem to call netr_LogonGetCapabilities with query_level=2 after a call with query_level=1. An unpatched Windows Server returns DCERPC_NCA_S_FAULT_INVALID_TAG for query_level values other than 1. While Samba tries to return NT_STATUS_NOT_SUPPORTED, but later fails to marshall the response, which results in DCERPC_FAULT_BAD_STUB_DATA instead. Because we don't have any documentation for level 2 yet, we just try to behave like an unpatched server and generate DCERPC_NCA_S_FAULT_INVALID_TAG instead of DCERPC_FAULT_BAD_STUB_DATA. Which allows patched Windows clients to keep working against a Samba DC. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15418 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit d5f1097b6220676d56ed5fc6707acf667b704518) --- .../knownfail.d/netr_LogonGetCapabilities | 2 -- source4/rpc_server/netlogon/dcerpc_netlogon.c | 28 ++++++++++++++++--- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/selftest/knownfail.d/netr_LogonGetCapabilities b/selftest/knownfail.d/netr_LogonGetCapabilities index 30aadf3bb9d..99c7ac711ed 100644 --- a/selftest/knownfail.d/netr_LogonGetCapabilities +++ b/selftest/knownfail.d/netr_LogonGetCapabilities @@ -1,3 +1 @@ ^samba3.rpc.schannel.*\.schannel\(nt4_dc -^samba3.rpc.schannel.*\.schannel\(ad_dc -^samba4.rpc.schannel.*\.schannel\(ad_dc diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c index 6a3e044eb9d..26be4f56751 100644 --- a/source4/rpc_server/netlogon/dcerpc_netlogon.c +++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c @@ -2399,6 +2399,30 @@ static NTSTATUS dcesrv_netr_LogonGetCapabilities(struct dcesrv_call_state *dce_c struct netlogon_creds_CredentialState *creds; NTSTATUS status; + switch (r->in.query_level) { + case 1: + break; + case 2: + /* + * Until we know the details behind KB5028166 + * just return DCERPC_NCA_S_FAULT_INVALID_TAG + * like an unpatched Windows Server. + */ + FALL_THROUGH; + default: + /* + * There would not be a way to marshall the + * the response. Which would mean our final + * ndr_push would fail an we would return + * an RPC-level fault with DCERPC_FAULT_BAD_STUB_DATA. + * + * But it's important to match a Windows server + * especially before KB5028166, see also our bug #15418 + * Otherwise Windows client would stop talking to us. + */ + DCESRV_FAULT(DCERPC_NCA_S_FAULT_INVALID_TAG); + } + status = dcesrv_netr_creds_server_step_check(dce_call, mem_ctx, r->in.computer_name, @@ -2410,10 +2434,6 @@ static NTSTATUS dcesrv_netr_LogonGetCapabilities(struct dcesrv_call_state *dce_c } NT_STATUS_NOT_OK_RETURN(status); - if (r->in.query_level != 1) { - return NT_STATUS_NOT_SUPPORTED; - } - r->out.capabilities->server_capabilities = creds->negotiate_flags; return NT_STATUS_OK; -- 2.34.1 From caf21883fa83a3277a1df405c32eeeda3950495b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Sat, 15 Jul 2023 16:11:48 +0200 Subject: [PATCH 23/25] s3:rpc_server:netlogon: generate FAULT_INVALID_TAG for invalid netr_LogonGetCapabilities levels This is important as Windows clients with KB5028166 seem to call netr_LogonGetCapabilities with query_level=2 after a call with query_level=1. An unpatched Windows Server returns DCERPC_NCA_S_FAULT_INVALID_TAG for query_level values other than 1. While Samba tries to return NT_STATUS_NOT_SUPPORTED, but later fails to marshall the response, which results in DCERPC_FAULT_BAD_STUB_DATA instead. Because we don't have any documentation for level 2 yet, we just try to behave like an unpatched server and generate DCERPC_NCA_S_FAULT_INVALID_TAG instead of DCERPC_FAULT_BAD_STUB_DATA. Which allows patched Windows clients to keep working against a Samba DC. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15418 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Mon Jul 17 07:35:09 UTC 2023 on atb-devel-224 (cherry picked from commit dfeabce44fbb78083fbbb2aa634fc4172cf83db9) --- .../knownfail.d/netr_LogonGetCapabilities | 1 - source3/rpc_server/netlogon/srv_netlog_nt.c | 29 ++++++++++++++++--- 2 files changed, 25 insertions(+), 5 deletions(-) delete mode 100644 selftest/knownfail.d/netr_LogonGetCapabilities diff --git a/selftest/knownfail.d/netr_LogonGetCapabilities b/selftest/knownfail.d/netr_LogonGetCapabilities deleted file mode 100644 index 99c7ac711ed..00000000000 --- a/selftest/knownfail.d/netr_LogonGetCapabilities +++ /dev/null @@ -1 +0,0 @@ -^samba3.rpc.schannel.*\.schannel\(nt4_dc diff --git a/source3/rpc_server/netlogon/srv_netlog_nt.c b/source3/rpc_server/netlogon/srv_netlog_nt.c index 7d17ab79f3d..b5c861edcf9 100644 --- a/source3/rpc_server/netlogon/srv_netlog_nt.c +++ b/source3/rpc_server/netlogon/srv_netlog_nt.c @@ -2286,6 +2286,31 @@ NTSTATUS _netr_LogonGetCapabilities(struct pipes_struct *p, struct netlogon_creds_CredentialState *creds; NTSTATUS status; + switch (r->in.query_level) { + case 1: + break; + case 2: + /* + * Until we know the details behind KB5028166 + * just return DCERPC_NCA_S_FAULT_INVALID_TAG + * like an unpatched Windows Server. + */ + FALL_THROUGH; + default: + /* + * There would not be a way to marshall the + * the response. Which would mean our final + * ndr_push would fail an we would return + * an RPC-level fault with DCERPC_FAULT_BAD_STUB_DATA. + * + * But it's important to match a Windows server + * especially before KB5028166, see also our bug #15418 + * Otherwise Windows client would stop talking to us. + */ + p->fault_state = DCERPC_NCA_S_FAULT_INVALID_TAG; + return NT_STATUS_NOT_SUPPORTED; + } + become_root(); status = dcesrv_netr_creds_server_step_check(p->dce_call, p->mem_ctx, @@ -2298,10 +2323,6 @@ NTSTATUS _netr_LogonGetCapabilities(struct pipes_struct *p, return status; } - if (r->in.query_level != 1) { - return NT_STATUS_NOT_SUPPORTED; - } - r->out.capabilities->server_capabilities = creds->negotiate_flags; return NT_STATUS_OK; -- 2.34.1 From 40053373794dba56762b184e6d34c2c937065f50 Mon Sep 17 00:00:00 2001 From: Jule Anger Date: Mon, 17 Jul 2023 21:39:40 +0200 Subject: [PATCH 24/25] WHATSNEW: Add release notes for Samba 4.16.11. Signed-off-by: Jule Anger --- WHATSNEW.txt | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 72 insertions(+), 2 deletions(-) diff --git a/WHATSNEW.txt b/WHATSNEW.txt index 4ddfe2db83c..2b6da8e411c 100644 --- a/WHATSNEW.txt +++ b/WHATSNEW.txt @@ -1,3 +1,74 @@ + =============================== + Release Notes for Samba 4.16.11 + July 19, 2023 + =============================== + + +This is a security release in order to address the following defects: + +o CVE-2022-2127: When winbind is used for NTLM authentication, a maliciously + crafted request can trigger an out-of-bounds read in winbind + and possibly crash it. + https://www.samba.org/samba/security/CVE-2022-2127.html + +o CVE-2023-34966: An infinite loop bug in Samba's mdssvc RPC service for + Spotlight can be triggered by an unauthenticated attacker by + issuing a malformed RPC request. + https://www.samba.org/samba/security/CVE-2023-34966.html + +o CVE-2023-34967: Missing type validation in Samba's mdssvc RPC service for + Spotlight can be used by an unauthenticated attacker to + trigger a process crash in a shared RPC mdssvc worker process. + https://www.samba.org/samba/security/CVE-2023-34967.html + +o CVE-2023-34968: As part of the Spotlight protocol Samba discloses the server- + side absolute path of shares and files and directories in + search results. + https://www.samba.org/samba/security/CVE-2023-34968.html + + +Changes since 4.16.10 +--------------------- + +o Ralph Boehme + * BUG 15072: CVE-2022-2127. + * BUG 15340: CVE-2023-34966. + * BUG 15341: CVE-2023-34967. + * BUG 15388: CVE-2023-34968. + +o Samuel Cabrero + * BUG 15072: CVE-2022-2127. + +o Volker Lendecke + * BUG 15072: CVE-2022-2127. + +o Stefan Metzmacher + * BUG 15418: Secure channel faulty since Windows 10/11 update 07/2023. + + +####################################### +Reporting bugs & Development Discussion +####################################### + +Please discuss this release on the samba-technical mailing list or by +joining the #samba-technical:matrix.org matrix room, or +#samba-technical IRC channel on irc.libera.chat. + +If you do report problems then please try to send high quality +feedback. If you don't provide vital information to help us track down +the problem then you will probably be ignored. All bug reports should +be filed under the Samba 4.1 and newer product in the project's Bugzilla +database (https://bugzilla.samba.org/). + + +====================================================================== +== Our Code, Our Bugs, Our Responsibility. +== The Samba Team +====================================================================== + + +Release notes for older releases follow: +---------------------------------------- =============================== Release Notes for Samba 4.16.10 March 29, 2023 @@ -56,8 +127,7 @@ database (https://bugzilla.samba.org/). ====================================================================== -Release notes for older releases follow: ----------------------------------------- +---------------------------------------------------------------------- ============================== Release Notes for Samba 4.16.9 February 16, 2023 -- 2.34.1 From 225a003a043eee399b6d266d94440c399b6877e4 Mon Sep 17 00:00:00 2001 From: Jule Anger Date: Mon, 17 Jul 2023 21:39:58 +0200 Subject: [PATCH 25/25] VERSION: Disable GIT_SNAPSHOT for the 4.16.11 release. Signed-off-by: Jule Anger --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 22fca36686e..3eb1e7e9d58 100644 --- a/VERSION +++ b/VERSION @@ -99,7 +99,7 @@ SAMBA_VERSION_RC_RELEASE= # e.g. SAMBA_VERSION_IS_SVN_SNAPSHOT=yes # # -> "3.0.0-SVN-build-199" # ######################################################## -SAMBA_VERSION_IS_GIT_SNAPSHOT=yes +SAMBA_VERSION_IS_GIT_SNAPSHOT=no ######################################################## # This is for specifying a release nickname # -- 2.34.1