| From a5da8919303ea99937c0d3b536f964b7f1addda7 Mon Sep 17 00:00:00 2001 |
| From: Jeremy Allison <jra@samba.org> |
| Date: Fri, 10 Jul 2020 15:09:33 -0700 |
| Subject: [PATCH 1/6] s4: torture: Add smb2.notify.handle-permissions test. |
| |
| Add knownfail entry. |
| |
| CVE-2020-14318 |
| |
| BUG: https://bugzilla.samba.org/show_bug.cgi?id=14434 |
| |
| Signed-off-by: Jeremy Allison <jra@samba.org> |
| --- |
| .../smb2_notify_handle_permissions | 2 + |
| source4/torture/smb2/notify.c | 80 +++++++++++++++++++ |
| 2 files changed, 82 insertions(+) |
| create mode 100644 selftest/knownfail.d/smb2_notify_handle_permissions |
| |
| diff --git a/selftest/knownfail.d/smb2_notify_handle_permissions b/selftest/knownfail.d/smb2_notify_handle_permissions |
| new file mode 100644 |
| index 00000000000..c0ec8fc8153 |
| --- /dev/null |
| +++ b/selftest/knownfail.d/smb2_notify_handle_permissions |
| @@ -0,0 +1,2 @@ |
| +^samba3.smb2.notify.handle-permissions |
| + |
| diff --git a/source4/torture/smb2/notify.c b/source4/torture/smb2/notify.c |
| index d8aa44f5d4c..79096394130 100644 |
| --- a/source4/torture/smb2/notify.c |
| +++ b/source4/torture/smb2/notify.c |
| @@ -2649,6 +2649,83 @@ done: |
| return ok; |
| } |
| |
| +/* |
| + Test asking for a change notify on a handle without permissions. |
| +*/ |
| + |
| +#define BASEDIR_HPERM BASEDIR "_HPERM" |
| + |
| +static bool torture_smb2_notify_handle_permissions( |
| + struct torture_context *torture, |
| + struct smb2_tree *tree) |
| +{ |
| + bool ret = true; |
| + NTSTATUS status; |
| + union smb_notify notify; |
| + union smb_open io; |
| + struct smb2_handle h1 = {{0}}; |
| + struct smb2_request *req; |
| + |
| + smb2_deltree(tree, BASEDIR_HPERM); |
| + smb2_util_rmdir(tree, BASEDIR_HPERM); |
| + |
| + torture_comment(torture, |
| + "TESTING CHANGE NOTIFY " |
| + "ON A HANDLE WITHOUT PERMISSIONS\n"); |
| + |
| + /* |
| + get a handle on the directory |
| + */ |
| + ZERO_STRUCT(io.smb2); |
| + io.generic.level = RAW_OPEN_SMB2; |
| + io.smb2.in.create_flags = 0; |
| + io.smb2.in.desired_access = SEC_FILE_READ_ATTRIBUTE; |
| + io.smb2.in.create_options = NTCREATEX_OPTIONS_DIRECTORY; |
| + io.smb2.in.file_attributes = FILE_ATTRIBUTE_NORMAL; |
| + io.smb2.in.share_access = NTCREATEX_SHARE_ACCESS_READ | |
| + NTCREATEX_SHARE_ACCESS_WRITE; |
| + io.smb2.in.alloc_size = 0; |
| + io.smb2.in.create_disposition = NTCREATEX_DISP_CREATE; |
| + io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS; |
| + io.smb2.in.security_flags = 0; |
| + io.smb2.in.fname = BASEDIR_HPERM; |
| + |
| + status = smb2_create(tree, torture, &io.smb2); |
| + CHECK_STATUS(status, NT_STATUS_OK); |
| + h1 = io.smb2.out.file.handle; |
| + |
| + /* ask for a change notify, |
| + on file or directory name changes */ |
| + ZERO_STRUCT(notify.smb2); |
| + notify.smb2.level = RAW_NOTIFY_SMB2; |
| + notify.smb2.in.buffer_size = 1000; |
| + notify.smb2.in.completion_filter = FILE_NOTIFY_CHANGE_NAME; |
| + notify.smb2.in.file.handle = h1; |
| + notify.smb2.in.recursive = true; |
| + |
| + req = smb2_notify_send(tree, ¬ify.smb2); |
| + torture_assert_goto(torture, |
| + req != NULL, |
| + ret, |
| + done, |
| + "smb2_notify_send failed\n"); |
| + |
| + /* |
| + * Cancel it, we don't really want to wait. |
| + */ |
| + smb2_cancel(req); |
| + status = smb2_notify_recv(req, torture, ¬ify.smb2); |
| + /* Handle h1 doesn't have permissions for ChangeNotify. */ |
| + CHECK_STATUS(status, NT_STATUS_ACCESS_DENIED); |
| + |
| +done: |
| + if (!smb2_util_handle_empty(h1)) { |
| + smb2_util_close(tree, h1); |
| + } |
| + smb2_deltree(tree, BASEDIR_HPERM); |
| + return ret; |
| +} |
| + |
| /* |
| basic testing of SMB2 change notify |
| */ |
| @@ -2682,6 +2759,9 @@ struct torture_suite *torture_smb2_notify_init(TALLOC_CTX *ctx) |
| torture_smb2_notify_rmdir3); |
| torture_suite_add_2smb2_test(suite, "rmdir4", |
| torture_smb2_notify_rmdir4); |
| + torture_suite_add_1smb2_test(suite, |
| + "handle-permissions", |
| + torture_smb2_notify_handle_permissions); |
| |
| suite->description = talloc_strdup(suite, "SMB2-NOTIFY tests"); |
| |
| -- |
| 2.17.1 |
| |
| |
| From c300a85848350635e7ddd8129b31c4d439dc0f8a Mon Sep 17 00:00:00 2001 |
| From: Jeremy Allison <jra@samba.org> |
| Date: Tue, 7 Jul 2020 18:25:23 -0700 |
| Subject: [PATCH 2/6] s3: smbd: Ensure change notifies can't get set unless the |
| directory handle is open for SEC_DIR_LIST. |
| |
| Remove knownfail entry. |
| |
| CVE-2020-14318 |
| |
| BUG: https://bugzilla.samba.org/show_bug.cgi?id=14434 |
| |
| Signed-off-by: Jeremy Allison <jra@samba.org> |
| --- |
| selftest/knownfail.d/smb2_notify_handle_permissions | 2 -- |
| source3/smbd/notify.c | 8 ++++++++ |
| 2 files changed, 8 insertions(+), 2 deletions(-) |
| delete mode 100644 selftest/knownfail.d/smb2_notify_handle_permissions |
| |
| diff --git a/selftest/knownfail.d/smb2_notify_handle_permissions b/selftest/knownfail.d/smb2_notify_handle_permissions |
| deleted file mode 100644 |
| index c0ec8fc8153..00000000000 |
| --- a/selftest/knownfail.d/smb2_notify_handle_permissions |
| +++ /dev/null |
| @@ -1,2 +0,0 @@ |
| -^samba3.smb2.notify.handle-permissions |
| - |
| diff --git a/source3/smbd/notify.c b/source3/smbd/notify.c |
| index b36a4c0003a..68553686fa2 100644 |
| --- a/source3/smbd/notify.c |
| +++ b/source3/smbd/notify.c |
| @@ -289,6 +289,14 @@ NTSTATUS change_notify_create(struct files_struct *fsp, |
| char fullpath[len+1]; |
| NTSTATUS status = NT_STATUS_NOT_IMPLEMENTED; |
| |
| + /* |
| + * Setting a changenotify needs READ/LIST access |
| + * on the directory handle. |
| + */ |
| + if (!(fsp->access_mask & SEC_DIR_LIST)) { |
| + return NT_STATUS_ACCESS_DENIED; |
| + } |
| + |
| if (fsp->notify != NULL) { |
| DEBUG(1, ("change_notify_create: fsp->notify != NULL, " |
| "fname = %s\n", fsp->fsp_name->base_name)); |
| -- |
| 2.17.1 |
| |
| |
| From e6fe5b4d64a8e1a03e1aaebafd97f313b3c94342 Mon Sep 17 00:00:00 2001 |
| From: Volker Lendecke <vl@samba.org> |
| Date: Thu, 9 Jul 2020 21:49:25 +0200 |
| Subject: [PATCH 3/6] CVE-2020-14323 winbind: Fix invalid lookupsids DoS |
| |
| A lookupsids request without extra_data will lead to "state->domain==NULL", |
| which makes winbindd_lookupsids_recv trying to dereference it. |
| |
| Reported by Bas Alberts of the GitHub Security Lab Team as GHSL-2020-134 |
| |
| Bug: https://bugzilla.samba.org/show_bug.cgi?id=14436 |
| Signed-off-by: Volker Lendecke <vl@samba.org> |
| --- |
| source3/winbindd/winbindd_lookupsids.c | 2 +- |
| 1 file changed, 1 insertion(+), 1 deletion(-) |
| |
| diff --git a/source3/winbindd/winbindd_lookupsids.c b/source3/winbindd/winbindd_lookupsids.c |
| index d28b5fa9f01..a289fd86f0f 100644 |
| --- a/source3/winbindd/winbindd_lookupsids.c |
| +++ b/source3/winbindd/winbindd_lookupsids.c |
| @@ -47,7 +47,7 @@ struct tevent_req *winbindd_lookupsids_send(TALLOC_CTX *mem_ctx, |
| DEBUG(3, ("lookupsids\n")); |
| |
| if (request->extra_len == 0) { |
| - tevent_req_done(req); |
| + tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); |
| return tevent_req_post(req, ev); |
| } |
| if (request->extra_data.data[request->extra_len-1] != '\0') { |
| -- |
| 2.17.1 |
| |
| |
| From 6093b2d815a00a577036fa001b47d7fc5514ad2c Mon Sep 17 00:00:00 2001 |
| From: Volker Lendecke <vl@samba.org> |
| Date: Thu, 9 Jul 2020 21:48:57 +0200 |
| Subject: [PATCH 4/6] CVE-2020-14323 torture4: Add a simple test for invalid |
| lookup_sids winbind call |
| |
| We can't add this test before the fix, add it to knownfail and have the fix |
| remove the knownfail entry again. As this crashes winbind, many tests after |
| this one will fail. |
| |
| Reported by Bas Alberts of the GitHub Security Lab Team as GHSL-2020-134 |
| |
| Bug: https://bugzilla.samba.org/show_bug.cgi?id=14436 |
| Signed-off-by: Volker Lendecke <vl@samba.org> |
| --- |
| source4/torture/winbind/struct_based.c | 27 ++++++++++++++++++++++++++ |
| 1 file changed, 27 insertions(+) |
| |
| diff --git a/source4/torture/winbind/struct_based.c b/source4/torture/winbind/struct_based.c |
| index 9745b621ca9..71f248c0d61 100644 |
| --- a/source4/torture/winbind/struct_based.c |
| +++ b/source4/torture/winbind/struct_based.c |
| @@ -1110,6 +1110,29 @@ static bool torture_winbind_struct_lookup_name_sid(struct torture_context *tortu |
| return true; |
| } |
| |
| +static bool torture_winbind_struct_lookup_sids_invalid( |
| + struct torture_context *torture) |
| +{ |
| + struct winbindd_request req = {0}; |
| + struct winbindd_response rep = {0}; |
| + bool strict = torture_setting_bool(torture, "strict mode", false); |
| + bool ok; |
| + |
| + torture_comment(torture, |
| + "Running WINBINDD_LOOKUP_SIDS (struct based)\n"); |
| + |
| + ok = true; |
| + DO_STRUCT_REQ_REP_EXT(WINBINDD_LOOKUPSIDS, &req, &rep, |
| + NSS_STATUS_NOTFOUND, |
| + strict, |
| + ok=false, |
| + talloc_asprintf( |
| + torture, |
| + "invalid lookupsids succeeded")); |
| + |
| + return ok; |
| +} |
| + |
| struct torture_suite *torture_winbind_struct_init(TALLOC_CTX *ctx) |
| { |
| struct torture_suite *suite = torture_suite_create(ctx, "struct"); |
| @@ -1132,6 +1155,10 @@ struct torture_suite *torture_winbind_struct_init(TALLOC_CTX *ctx) |
| torture_suite_add_simple_test(suite, "getpwent", torture_winbind_struct_getpwent); |
| torture_suite_add_simple_test(suite, "endpwent", torture_winbind_struct_endpwent); |
| torture_suite_add_simple_test(suite, "lookup_name_sid", torture_winbind_struct_lookup_name_sid); |
| + torture_suite_add_simple_test( |
| + suite, |
| + "lookup_sids_invalid", |
| + torture_winbind_struct_lookup_sids_invalid); |
| |
| suite->description = talloc_strdup(suite, "WINBIND - struct based protocol tests"); |
| |
| -- |
| 2.17.1 |
| |
| |
| From 2632e8ebae826a7305fe7d3948ee28b77d2ffbc0 Mon Sep 17 00:00:00 2001 |
| From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> |
| Date: Fri, 21 Aug 2020 17:10:22 +1200 |
| Subject: [PATCH 5/6] CVE-2020-14383: s4/dns: Ensure variable initialization |
| with NULL. |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| Based on patches from Francis Brosnan Blázquez <francis@aspl.es> |
| and Jeremy Allison <jra@samba.org> |
| |
| BUG: https://bugzilla.samba.org/show_bug.cgi?id=14472 |
| BUG: https://bugzilla.samba.org/show_bug.cgi?id=12795 |
| |
| Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> |
| Reviewed-by: Jeremy Allison <jra@samba.org> |
| (based on commit 7afe449e7201be92bed8e53cbb37b74af720ef4e) |
| --- |
| .../rpc_server/dnsserver/dcerpc_dnsserver.c | 24 ++++++++++--------- |
| 1 file changed, 13 insertions(+), 11 deletions(-) |
| |
| diff --git a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c |
| index b6389f2328a..ec610168266 100644 |
| --- a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c |
| +++ b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c |
| @@ -1759,15 +1759,17 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate, |
| TALLOC_CTX *tmp_ctx; |
| char *name; |
| const char * const attrs[] = { "name", "dnsRecord", NULL }; |
| - struct ldb_result *res; |
| - struct DNS_RPC_RECORDS_ARRAY *recs; |
| + struct ldb_result *res = NULL; |
| + struct DNS_RPC_RECORDS_ARRAY *recs = NULL; |
| char **add_names = NULL; |
| - char *rname; |
| + char *rname = NULL; |
| const char *preference_name = NULL; |
| int add_count = 0; |
| int i, ret, len; |
| WERROR status; |
| - struct dns_tree *tree, *base, *node; |
| + struct dns_tree *tree = NULL; |
| + struct dns_tree *base = NULL; |
| + struct dns_tree *node = NULL; |
| |
| tmp_ctx = talloc_new(mem_ctx); |
| W_ERROR_HAVE_NO_MEMORY(tmp_ctx); |
| @@ -1850,9 +1852,9 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate, |
| } |
| } |
| |
| - talloc_free(res); |
| - talloc_free(tree); |
| - talloc_free(name); |
| + TALLOC_FREE(res); |
| + TALLOC_FREE(tree); |
| + TALLOC_FREE(name); |
| |
| /* Add any additional records */ |
| if (select_flag & DNS_RPC_VIEW_ADDITIONAL_DATA) { |
| @@ -1870,14 +1872,14 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate, |
| LDB_SCOPE_ONELEVEL, attrs, |
| "(&(objectClass=dnsNode)(name=%s)(!(dNSTombstoned=TRUE)))", |
| encoded_name); |
| - talloc_free(name); |
| + TALLOC_FREE(name); |
| if (ret != LDB_SUCCESS) { |
| continue; |
| } |
| if (res->count == 1) { |
| break; |
| } else { |
| - talloc_free(res); |
| + TALLOC_FREE(res); |
| continue; |
| } |
| } |
| @@ -1892,8 +1894,8 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate, |
| select_flag, rname, |
| res->msgs[0], 0, recs, |
| NULL, NULL); |
| - talloc_free(rname); |
| - talloc_free(res); |
| + TALLOC_FREE(rname); |
| + TALLOC_FREE(res); |
| if (!W_ERROR_IS_OK(status)) { |
| talloc_free(tmp_ctx); |
| return status; |
| -- |
| 2.17.1 |
| |
| |
| From 8e09649351e9e8143b4bd0b76bcbd2cfb4d2f281 Mon Sep 17 00:00:00 2001 |
| From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> |
| Date: Fri, 21 Aug 2020 17:23:17 +1200 |
| Subject: [PATCH 6/6] CVE-2020-14383: s4/dns: do not crash when additional data |
| not found |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| Found by Francis Brosnan Blázquez <francis@aspl.es>. |
| |
| BUG: https://bugzilla.samba.org/show_bug.cgi?id=14472 |
| BUG: https://bugzilla.samba.org/show_bug.cgi?id=12795 |
| |
| Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> |
| Reviewed-by: Jeremy Allison <jra@samba.org> |
| |
| Autobuild-User(master): Douglas Bagnall <dbagnall@samba.org> |
| Autobuild-Date(master): Mon Aug 24 00:21:41 UTC 2020 on sn-devel-184 |
| |
| (based on commit df98e7db04c901259dd089e20cd557bdbdeaf379) |
| --- |
| source4/rpc_server/dnsserver/dcerpc_dnsserver.c | 7 ++++--- |
| 1 file changed, 4 insertions(+), 3 deletions(-) |
| |
| diff --git a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c |
| index ec610168266..88efc01f154 100644 |
| --- a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c |
| +++ b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c |
| @@ -1859,8 +1859,8 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate, |
| /* Add any additional records */ |
| if (select_flag & DNS_RPC_VIEW_ADDITIONAL_DATA) { |
| for (i=0; i<add_count; i++) { |
| - struct dnsserver_zone *z2; |
| - |
| + struct dnsserver_zone *z2 = NULL; |
| + struct ldb_message *msg = NULL; |
| /* Search all the available zones for additional name */ |
| for (z2 = dsstate->zones; z2; z2 = z2->next) { |
| char *encoded_name; |
| @@ -1877,6 +1877,7 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate, |
| continue; |
| } |
| if (res->count == 1) { |
| + msg = res->msgs[0]; |
| break; |
| } else { |
| TALLOC_FREE(res); |
| @@ -1892,7 +1893,7 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate, |
| } |
| status = dns_fill_records_array(tmp_ctx, NULL, DNS_TYPE_A, |
| select_flag, rname, |
| - res->msgs[0], 0, recs, |
| + msg, 0, recs, |
| NULL, NULL); |
| TALLOC_FREE(rname); |
| TALLOC_FREE(res); |
| -- |
| 2.17.1 |
| |