From d392b10c55bbcedda01fdd87fe6035fa3a6986b3 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 18 Jan 2022 11:56:38 +1300 Subject: [PATCH 01/99] CVE-2022-0336: pytest: Add a test for an SPN conflict with a re-added SPN This test currently fails, as re-adding an SPN means that later checks do not run. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14950 Signed-off-by: Joseph Sutton Reviewed-by: Douglas Bagnall --- python/samba/tests/ldap_spn.py | 7 +++++++ selftest/knownfail.d/ldap_spn | 1 + 2 files changed, 8 insertions(+) diff --git a/python/samba/tests/ldap_spn.py b/python/samba/tests/ldap_spn.py index 8a398ffaa49..6ebdf8f9a32 100644 --- a/python/samba/tests/ldap_spn.py +++ b/python/samba/tests/ldap_spn.py @@ -268,6 +268,8 @@ class LdapSpnTestBase(TestCase): for k in ('dNSHostName', 'servicePrincipalName'): if isinstance(m.get(k), str): m[k] = m[k].format(dnsname=f"x.{REALM}") + elif isinstance(m.get(k), list): + m[k] = [x.format(dnsname=f"x.{REALM}") for x in m[k]] msg = ldb.Message.from_dict(samdb, m, op) @@ -727,6 +729,11 @@ class LdapSpnSambaOnlyTest(LdapSpnTestBase): ('user:C', 'host/{dnsname}', '*', ok), ('user:D', 'www/{dnsname}', 'D', denied), ), + ("add a conflict, along with a re-added SPN", + ('A', 'cifs/{dnsname}', '*', ok), + ('B', 'cifs/heeble.example.net', 'B', ok), + ('B', ['cifs/heeble.example.net', 'host/{dnsname}'], 'B', constraint), + ), ("changing dNSHostName after host", ('A', {'dNSHostName': '{dnsname}'}, '*', ok), diff --git a/selftest/knownfail.d/ldap_spn b/selftest/knownfail.d/ldap_spn index 63f9fe02ef7..16dafa91b66 100644 --- a/selftest/knownfail.d/ldap_spn +++ b/selftest/knownfail.d/ldap_spn @@ -1 +1,2 @@ samba.tests.ldap_spn.+LdapSpnTest.test_spn_dodgy_spns +samba.tests.ldap_spn.+LdapSpnSambaOnlyTest.test_spn_add_a_conflict_along_with_a_re_added_SPN -- 2.25.1 From 7a516257ea310fa045bdf14e677eaa97f2a83c33 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 18 Jan 2022 12:02:45 +1300 Subject: [PATCH 02/99] CVE-2022-0336: s4/dsdb/samldb: Don't return early when an SPN is re-added to an object If an added SPN already exists on an object, we still want to check the rest of the element values for conflicts. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14950 Signed-off-by: Joseph Sutton Reviewed-by: Douglas Bagnall --- selftest/knownfail.d/ldap_spn | 1 - source4/dsdb/samdb/ldb_modules/samldb.c | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/selftest/knownfail.d/ldap_spn b/selftest/knownfail.d/ldap_spn index 16dafa91b66..63f9fe02ef7 100644 --- a/selftest/knownfail.d/ldap_spn +++ b/selftest/knownfail.d/ldap_spn @@ -1,2 +1 @@ samba.tests.ldap_spn.+LdapSpnTest.test_spn_dodgy_spns -samba.tests.ldap_spn.+LdapSpnSambaOnlyTest.test_spn_add_a_conflict_along_with_a_re_added_SPN diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c index db3883eb527..24971d521aa 100644 --- a/source4/dsdb/samdb/ldb_modules/samldb.c +++ b/source4/dsdb/samdb/ldb_modules/samldb.c @@ -4006,8 +4006,7 @@ static int samldb_spn_uniqueness_check(struct samldb_ctx *ac, ac->msg->dn); if (ret == LDB_ERR_COMPARE_TRUE) { DBG_INFO("SPN %s re-added to the same object\n", spn); - talloc_free(tmp_ctx); - return LDB_SUCCESS; + continue; } if (ret != LDB_SUCCESS) { DBG_ERR("SPN %s failed direct uniqueness check\n", spn); -- 2.25.1 From eee61be9b5867b63b73b0b1fea03f44a4e1235b7 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 13 Jan 2022 16:48:01 +0100 Subject: [PATCH 03/99] CVE-2021-44142: libadouble: add defines for icon lengths From https://www.ietf.org/rfc/rfc1740.txt BUG: https://bugzilla.samba.org/show_bug.cgi?id=14914 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison --- source3/lib/adouble.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/source3/lib/adouble.h b/source3/lib/adouble.h index 8b14d0ab871..de44f3f5fdc 100644 --- a/source3/lib/adouble.h +++ b/source3/lib/adouble.h @@ -101,6 +101,8 @@ typedef enum {ADOUBLE_META, ADOUBLE_RSRC} adouble_type_t; #define ADEDLEN_MACFILEI 4 #define ADEDLEN_PRODOSFILEI 8 #define ADEDLEN_MSDOSFILEI 2 +#define ADEDLEN_ICONBW 128 +#define ADEDLEN_ICONCOL 1024 #define ADEDLEN_DID 4 #define ADEDLEN_PRIVDEV 8 #define ADEDLEN_PRIVINO 8 -- 2.25.1 From 22b4091924977f6437b59627f33a8e6f02b41011 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 20 Nov 2021 16:36:42 +0100 Subject: [PATCH 04/99] CVE-2021-44142: smbd: add Netatalk xattr used by vfs_fruit to the list of private Samba xattrs This is an internal xattr that should not be user visible. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14914 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison --- source3/smbd/trans2.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index a86ac3228e3..4f6d92955cf 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -46,6 +46,7 @@ #include "libcli/smb/smb2_posix.h" #include "lib/util/string_wrappers.h" #include "source3/lib/substitute.h" +#include "source3/lib/adouble.h" #define DIR_ENTRY_SAFETY_MARGIN 4096 @@ -203,6 +204,7 @@ bool samba_private_attr_name(const char *unix_ea_name) SAMBA_XATTR_DOS_ATTRIB, SAMBA_XATTR_MARKER, XATTR_NTACL_NAME, + AFPINFO_EA_NETATALK, NULL }; -- 2.25.1 From b4c0b4620f12055207adb0519c8d91c3021f354a Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 26 Nov 2021 07:19:32 +0100 Subject: [PATCH 05/99] CVE-2021-44142: libadouble: harden ad_unpack_xattrs() This ensures ad_unpack_xattrs() is only called for an ad_type of ADOUBLE_RSRC, which is used for parsing ._ AppleDouble sidecar files, and the buffer ad->ad_data is AD_XATTR_MAX_HDR_SIZE bytes large which is a prerequisite for all buffer out-of-bounds access checks in ad_unpack_xattrs(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14914 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison --- source3/lib/adouble.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/source3/lib/adouble.c b/source3/lib/adouble.c index f809a445081..6cbe8a5aeda 100644 --- a/source3/lib/adouble.c +++ b/source3/lib/adouble.c @@ -707,14 +707,27 @@ static bool ad_pack(struct vfs_handle_struct *handle, static bool ad_unpack_xattrs(struct adouble *ad) { struct ad_xattr_header *h = &ad->adx_header; + size_t bufsize = talloc_get_size(ad->ad_data); const char *p = ad->ad_data; uint32_t hoff; uint32_t i; + if (ad->ad_type != ADOUBLE_RSRC) { + return false; + } + if (ad_getentrylen(ad, ADEID_FINDERI) <= ADEDLEN_FINDERI) { return true; } + /* + * Ensure the buffer ad->ad_data was allocated by ad_alloc() for an + * ADOUBLE_RSRC type (._ AppleDouble file on-disk). + */ + if (bufsize != AD_XATTR_MAX_HDR_SIZE) { + return false; + } + /* 2 bytes padding */ hoff = ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI + 2; @@ -964,9 +977,11 @@ static bool ad_unpack(struct adouble *ad, const size_t nentries, ad->ad_eid[eid].ade_len = len; } - ok = ad_unpack_xattrs(ad); - if (!ok) { - return false; + if (ad->ad_type == ADOUBLE_RSRC) { + ok = ad_unpack_xattrs(ad); + if (!ok) { + return false; + } } return true; -- 2.25.1 From 4533a7b4319cd95815d2dcd5fe5075539fb850e5 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 25 Nov 2021 15:04:03 +0100 Subject: [PATCH 06/99] CVE-2021-44142: libadouble: add basic cmocka tests BUG: https://bugzilla.samba.org/show_bug.cgi?id=14914 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison [slow@samba.org: conflict due to missing test in selftest/tests.py] --- selftest/knownfail.d/samba.unittests.adouble | 3 + selftest/tests.py | 2 + source3/lib/test_adouble.c | 389 +++++++++++++++++++ source3/wscript_build | 5 + 4 files changed, 399 insertions(+) create mode 100644 selftest/knownfail.d/samba.unittests.adouble create mode 100644 source3/lib/test_adouble.c diff --git a/selftest/knownfail.d/samba.unittests.adouble b/selftest/knownfail.d/samba.unittests.adouble new file mode 100644 index 00000000000..8b0314f2fae --- /dev/null +++ b/selftest/knownfail.d/samba.unittests.adouble @@ -0,0 +1,3 @@ +^samba.unittests.adouble.parse_abouble_finderinfo2\(none\) +^samba.unittests.adouble.parse_abouble_finderinfo3\(none\) +^samba.unittests.adouble.parse_abouble_date2\(none\) diff --git a/selftest/tests.py b/selftest/tests.py index e7338985caf..c87b41c1a66 100644 --- a/selftest/tests.py +++ b/selftest/tests.py @@ -434,3 +434,5 @@ if with_elasticsearch_backend: [os.path.join(bindir(), "default/source3/test_mdsparser_es")] + [configuration]) plantestsuite("samba.unittests.credentials", "none", [os.path.join(bindir(), "default/auth/credentials/test_creds")]) +plantestsuite("samba.unittests.adouble", "none", + [os.path.join(bindir(), "test_adouble")]) diff --git a/source3/lib/test_adouble.c b/source3/lib/test_adouble.c new file mode 100644 index 00000000000..615c22469c9 --- /dev/null +++ b/source3/lib/test_adouble.c @@ -0,0 +1,389 @@ +/* + * Unix SMB/CIFS implementation. + * + * Copyright (C) 2021 Ralph Boehme + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "adouble.c" +#include + +static int setup_talloc_context(void **state) +{ + TALLOC_CTX *frame = talloc_stackframe(); + + *state = frame; + return 0; +} + +static int teardown_talloc_context(void **state) +{ + TALLOC_CTX *frame = *state; + + TALLOC_FREE(frame); + return 0; +} + +/* + * Basic and sane buffer. + */ +static uint8_t ad_basic[] = { + 0x00, 0x05, 0x16, 0x07, /* Magic */ + 0x00, 0x02, 0x00, 0x00, /* Version */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x02, /* Count */ + /* adentry 1: FinderInfo */ + 0x00, 0x00, 0x00, 0x09, /* eid: FinderInfo */ + 0x00, 0x00, 0x00, 0x32, /* offset */ + 0x00, 0x00, 0x00, 0x20, /* length */ + /* adentry 2: Resourcefork */ + 0x00, 0x00, 0x00, 0x02, /* eid: Resourcefork */ + 0x00, 0x00, 0x00, 0x52, /* offset */ + 0xff, 0xff, 0xff, 0x00, /* length */ + /* FinderInfo data: 32 bytes */ + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, +}; + +/* + * An empty FinderInfo entry. + */ +static uint8_t ad_finderinfo1[] = { + 0x00, 0x05, 0x16, 0x07, /* Magic */ + 0x00, 0x02, 0x00, 0x00, /* Version */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x02, /* Count */ + /* adentry 1: FinderInfo */ + 0x00, 0x00, 0x00, 0x09, /* eid: FinderInfo */ + 0x00, 0x00, 0x00, 0x52, /* off: points at end of buffer */ + 0x00, 0x00, 0x00, 0x00, /* len: 0, so off+len don't exceed bufferlen */ + /* adentry 2: Resourcefork */ + 0x00, 0x00, 0x00, 0x02, /* eid: Resourcefork */ + 0x00, 0x00, 0x00, 0x52, /* offset */ + 0xff, 0xff, 0xff, 0x00, /* length */ + /* FinderInfo data: 32 bytes */ + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, +}; + +/* + * A dangerous FinderInfo with correct length exceeding buffer by one byte. + */ +static uint8_t ad_finderinfo2[] = { + 0x00, 0x05, 0x16, 0x07, /* Magic */ + 0x00, 0x02, 0x00, 0x00, /* Version */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x02, /* Count */ + /* adentry 1: FinderInfo */ + 0x00, 0x00, 0x00, 0x09, /* eid: FinderInfo */ + 0x00, 0x00, 0x00, 0x33, /* off: points at beginng of data + 1 */ + 0x00, 0x00, 0x00, 0x20, /* len: 32, so off+len exceeds bufferlen by 1 */ + /* adentry 2: Resourcefork */ + 0x00, 0x00, 0x00, 0x02, /* eid: Resourcefork */ + 0x00, 0x00, 0x00, 0x52, /* offset */ + 0xff, 0xff, 0xff, 0x00, /* length */ + /* FinderInfo data: 32 bytes */ + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, +}; + +static uint8_t ad_finderinfo3[] = { + 0x00, 0x05, 0x16, 0x07, /* Magic */ + 0x00, 0x02, 0x00, 0x00, /* Version */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x02, /* Count */ + /* adentry 1: FinderInfo */ + 0x00, 0x00, 0x00, 0x09, /* eid: FinderInfo */ + 0x00, 0x00, 0x00, 0x33, /* off: points at beginng of data + 1 */ + 0x00, 0x00, 0x00, 0x1f, /* len: 31, so off+len don't exceed buf */ + /* adentry 2: Resourcefork */ + 0x00, 0x00, 0x00, 0x02, /* eid: Resourcefork */ + 0x00, 0x00, 0x00, 0x52, /* offset */ + 0xff, 0xff, 0xff, 0x00, /* length */ + /* FinderInfo data: 32 bytes */ + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, +}; + +/* + * A dangerous name entry. + */ +static uint8_t ad_name[] = { + 0x00, 0x05, 0x16, 0x07, /* Magic */ + 0x00, 0x02, 0x00, 0x00, /* Version */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x02, /* Count */ + /* adentry 1: FinderInfo */ + 0x00, 0x00, 0x00, 0x09, /* eid: FinderInfo */ + 0x00, 0x00, 0x00, 0x32, /* offset */ + 0x00, 0x00, 0x00, 0x20, /* length */ + /* adentry 2: Name */ + 0x00, 0x00, 0x00, 0x03, /* eid: Name */ + 0x00, 0x00, 0x00, 0x52, /* off: points at end of buffer */ + 0x00, 0x00, 0x00, 0x01, /* len: 1, so off+len exceeds bufferlen */ + /* FinderInfo data: 32 bytes */ + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, +}; + +/* + * A empty ADEID_FILEDATESI entry. + */ +static uint8_t ad_date1[] = { + 0x00, 0x05, 0x16, 0x07, /* Magic */ + 0x00, 0x02, 0x00, 0x00, /* Version */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x02, /* Count */ + /* adentry 1: FinderInfo */ + 0x00, 0x00, 0x00, 0x09, /* eid: FinderInfo */ + 0x00, 0x00, 0x00, 0x32, /* offset */ + 0x00, 0x00, 0x00, 0x20, /* length */ + /* adentry 2: Dates */ + 0x00, 0x00, 0x00, 0x08, /* eid: dates */ + 0x00, 0x00, 0x00, 0x52, /* off: end of buffer */ + 0x00, 0x00, 0x00, 0x00, /* len: 0, empty entry, valid */ + /* FinderInfo data: 32 bytes */ + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, +}; + +/* + * A dangerous ADEID_FILEDATESI entry, invalid length. + */ +static uint8_t ad_date2[] = { + 0x00, 0x05, 0x16, 0x07, /* Magic */ + 0x00, 0x02, 0x00, 0x00, /* Version */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x00, 0x00, 0x00, /* Filler */ + 0x00, 0x02, /* Count */ + /* adentry 1: FinderInfo */ + 0x00, 0x00, 0x00, 0x09, /* eid: FinderInfo */ + 0x00, 0x00, 0x00, 0x32, /* offset */ + 0x00, 0x00, 0x00, 0x20, /* length */ + /* adentry 2: Dates */ + 0x00, 0x00, 0x00, 0x08, /* eid: dates */ + 0x00, 0x00, 0x00, 0x43, /* off: FinderInfo buf but one byte short */ + 0x00, 0x00, 0x00, 0x0f, /* len: 15, so off+len don't exceed bufferlen */ + /* FinderInfo data: 32 bytes */ + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, +}; + +static struct adouble *parse_adouble(TALLOC_CTX *mem_ctx, + uint8_t *adbuf, + size_t adsize, + off_t filesize) +{ + struct adouble *ad = NULL; + bool ok; + + ad = talloc_zero(mem_ctx, struct adouble); + ad->ad_data = talloc_zero_size(ad, adsize); + assert_non_null(ad); + + memcpy(ad->ad_data, adbuf, adsize); + + ok = ad_unpack(ad, 2, filesize); + if (!ok) { + return NULL; + } + + return ad; +} + +static void parse_abouble_basic(void **state) +{ + TALLOC_CTX *frame = *state; + struct adouble *ad = NULL; + char *p = NULL; + + ad = parse_adouble(frame, ad_basic, sizeof(ad_basic), 0xffffff52); + assert_non_null(ad); + + p = ad_get_entry(ad, ADEID_FINDERI); + assert_non_null(p); + + return; +} + +static void parse_abouble_finderinfo1(void **state) +{ + TALLOC_CTX *frame = *state; + struct adouble *ad = NULL; + char *p = NULL; + + ad = parse_adouble(frame, + ad_finderinfo1, + sizeof(ad_finderinfo1), + 0xffffff52); + assert_non_null(ad); + + p = ad_get_entry(ad, ADEID_FINDERI); + assert_null(p); + + return; +} + +static void parse_abouble_finderinfo2(void **state) +{ + TALLOC_CTX *frame = *state; + struct adouble *ad = NULL; + + ad = parse_adouble(frame, + ad_finderinfo2, + sizeof(ad_finderinfo2), + 0xffffff52); + assert_null(ad); + + return; +} + +static void parse_abouble_finderinfo3(void **state) +{ + TALLOC_CTX *frame = *state; + struct adouble *ad = NULL; + + ad = parse_adouble(frame, + ad_finderinfo3, + sizeof(ad_finderinfo3), + 0xffffff52); + assert_null(ad); + + return; +} + +static void parse_abouble_name(void **state) +{ + TALLOC_CTX *frame = *state; + struct adouble *ad = NULL; + + ad = parse_adouble(frame, ad_name, sizeof(ad_name), 0x52); + assert_null(ad); + + return; +} + +static void parse_abouble_date1(void **state) +{ + TALLOC_CTX *frame = *state; + struct adouble *ad = NULL; + char *p = NULL; + + ad = parse_adouble(frame, ad_date1, sizeof(ad_date1), 0x52); + assert_non_null(ad); + + p = ad_get_entry(ad, ADEID_FILEDATESI); + assert_null(p); + + return; +} + +static void parse_abouble_date2(void **state) +{ + TALLOC_CTX *frame = *state; + struct adouble *ad = NULL; + + ad = parse_adouble(frame, ad_date2, sizeof(ad_date2), 0x52); + assert_null(ad); + + return; +} + +int main(int argc, char *argv[]) +{ + int rc; + const struct CMUnitTest tests[] = { + cmocka_unit_test(parse_abouble_basic), + cmocka_unit_test(parse_abouble_finderinfo1), + cmocka_unit_test(parse_abouble_finderinfo2), + cmocka_unit_test(parse_abouble_finderinfo3), + cmocka_unit_test(parse_abouble_name), + cmocka_unit_test(parse_abouble_date1), + cmocka_unit_test(parse_abouble_date2), + }; + + if (argc == 2) { + cmocka_set_test_filter(argv[1]); + } + cmocka_set_message_output(CM_OUTPUT_SUBUNIT); + + rc = cmocka_run_group_tests(tests, + setup_talloc_context, + teardown_talloc_context); + + return rc; +} diff --git a/source3/wscript_build b/source3/wscript_build index 69febb53750..9df9bdd35b7 100644 --- a/source3/wscript_build +++ b/source3/wscript_build @@ -1085,6 +1085,11 @@ bld.SAMBA3_SUBSYSTEM('ADOUBLE', source='lib/adouble.c', deps='STRING_REPLACE') +bld.SAMBA3_BINARY('test_adouble', + source='lib/test_adouble.c', + deps='smbd_base STRING_REPLACE cmocka', + for_selftest=True) + bld.SAMBA3_SUBSYSTEM('STRING_REPLACE', source='lib/string_replace.c') -- 2.25.1 From 0e2b3fb982d1f53d111e10d9197ed2ec2e13712c Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 13 Jan 2022 17:03:02 +0100 Subject: [PATCH 07/99] CVE-2021-44142: libadouble: harden parsing code BUG: https://bugzilla.samba.org/show_bug.cgi?id=14914 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison --- selftest/knownfail.d/samba.unittests.adouble | 3 - source3/lib/adouble.c | 115 ++++++++++++++++--- 2 files changed, 101 insertions(+), 17 deletions(-) delete mode 100644 selftest/knownfail.d/samba.unittests.adouble diff --git a/selftest/knownfail.d/samba.unittests.adouble b/selftest/knownfail.d/samba.unittests.adouble deleted file mode 100644 index 8b0314f2fae..00000000000 --- a/selftest/knownfail.d/samba.unittests.adouble +++ /dev/null @@ -1,3 +0,0 @@ -^samba.unittests.adouble.parse_abouble_finderinfo2\(none\) -^samba.unittests.adouble.parse_abouble_finderinfo3\(none\) -^samba.unittests.adouble.parse_abouble_date2\(none\) diff --git a/source3/lib/adouble.c b/source3/lib/adouble.c index 6cbe8a5aeda..37fb686f17b 100644 --- a/source3/lib/adouble.c +++ b/source3/lib/adouble.c @@ -269,6 +269,95 @@ size_t ad_setentryoff(struct adouble *ad, int eid, size_t off) return ad->ad_eid[eid].ade_off = off; } +/* + * All entries besides FinderInfo and resource fork must fit into the + * buffer. FinderInfo is special as it may be larger then the default 32 bytes + * if it contains marshalled xattrs, which we will fixup that in + * ad_convert(). The first 32 bytes however must also be part of the buffer. + * + * The resource fork is never accessed directly by the ad_data buf. + */ +static bool ad_entry_check_size(uint32_t eid, + size_t bufsize, + uint32_t off, + uint32_t got_len) +{ + struct { + off_t expected_len; + bool fixed_size; + bool minimum_size; + } ad_checks[] = { + [ADEID_DFORK] = {-1, false, false}, /* not applicable */ + [ADEID_RFORK] = {-1, false, false}, /* no limit */ + [ADEID_NAME] = {ADEDLEN_NAME, false, false}, + [ADEID_COMMENT] = {ADEDLEN_COMMENT, false, false}, + [ADEID_ICONBW] = {ADEDLEN_ICONBW, true, false}, + [ADEID_ICONCOL] = {ADEDLEN_ICONCOL, false, false}, + [ADEID_FILEI] = {ADEDLEN_FILEI, true, false}, + [ADEID_FILEDATESI] = {ADEDLEN_FILEDATESI, true, false}, + [ADEID_FINDERI] = {ADEDLEN_FINDERI, false, true}, + [ADEID_MACFILEI] = {ADEDLEN_MACFILEI, true, false}, + [ADEID_PRODOSFILEI] = {ADEDLEN_PRODOSFILEI, true, false}, + [ADEID_MSDOSFILEI] = {ADEDLEN_MSDOSFILEI, true, false}, + [ADEID_SHORTNAME] = {ADEDLEN_SHORTNAME, false, false}, + [ADEID_AFPFILEI] = {ADEDLEN_AFPFILEI, true, false}, + [ADEID_DID] = {ADEDLEN_DID, true, false}, + [ADEID_PRIVDEV] = {ADEDLEN_PRIVDEV, true, false}, + [ADEID_PRIVINO] = {ADEDLEN_PRIVINO, true, false}, + [ADEID_PRIVSYN] = {ADEDLEN_PRIVSYN, true, false}, + [ADEID_PRIVID] = {ADEDLEN_PRIVID, true, false}, + }; + + if (eid >= ADEID_MAX) { + return false; + } + if (got_len == 0) { + /* Entry present, but empty, allow */ + return true; + } + if (ad_checks[eid].expected_len == 0) { + /* + * Shouldn't happen: implicitly initialized to zero because + * explicit initializer missing. + */ + return false; + } + if (ad_checks[eid].expected_len == -1) { + /* Unused or no limit */ + return true; + } + if (ad_checks[eid].fixed_size) { + if (ad_checks[eid].expected_len != got_len) { + /* Wrong size fo fixed size entry. */ + return false; + } + } else { + if (ad_checks[eid].minimum_size) { + if (got_len < ad_checks[eid].expected_len) { + /* + * Too small for variable sized entry with + * minimum size. + */ + return false; + } + } else { + if (got_len > ad_checks[eid].expected_len) { + /* Too big for variable sized entry. */ + return false; + } + } + } + if (off + got_len < off) { + /* wrap around */ + return false; + } + if (off + got_len > bufsize) { + /* overflow */ + return false; + } + return true; +} + /** * Return a pointer to an AppleDouble entry * @@ -276,8 +365,15 @@ size_t ad_setentryoff(struct adouble *ad, int eid, size_t off) **/ char *ad_get_entry(const struct adouble *ad, int eid) { + size_t bufsize = talloc_get_size(ad->ad_data); off_t off = ad_getentryoff(ad, eid); size_t len = ad_getentrylen(ad, eid); + bool valid; + + valid = ad_entry_check_size(eid, bufsize, off, len); + if (!valid) { + return NULL; + } if (off == 0 || len == 0) { return NULL; @@ -914,20 +1010,11 @@ static bool ad_unpack(struct adouble *ad, const size_t nentries, return false; } - /* - * All entries besides FinderInfo and resource fork - * must fit into the buffer. FinderInfo is special as - * it may be larger then the default 32 bytes (if it - * contains marshalled xattrs), but we will fixup that - * in ad_convert(). And the resource fork is never - * accessed directly by the ad_data buf (also see - * comment above) anyway. - */ - if ((eid != ADEID_RFORK) && - (eid != ADEID_FINDERI) && - ((off + len) > bufsize)) { - DEBUG(1, ("bogus eid %d: off: %" PRIu32 ", len: %" PRIu32 "\n", - eid, off, len)); + ok = ad_entry_check_size(eid, bufsize, off, len); + if (!ok) { + DBG_ERR("bogus eid [%"PRIu32"] bufsize [%zu] " + "off [%"PRIu32"] len [%"PRIu32"]\n", + eid, bufsize, off, len); return false; } -- 2.25.1 From 550ece56400dc7391296943cf93ce0a4e54f9843 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 12:05:51 -0800 Subject: [PATCH 08/99] CVE-2021-44141: s4: libcli: Add smbcli_unlink_wcard(). We will use this in place of smbcli_unlink() when we know we are using a wildcard pattern. If can be used to generally replace smbcli_unlink() as it calls down to smbcli_unlink() is no wildcard is detected. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/libcli/clifile.c | 96 ++++++++++++++++++++++++++++++++++++++++ source4/libcli/libcli.h | 5 +++ 2 files changed, 101 insertions(+) diff --git a/source4/libcli/clifile.c b/source4/libcli/clifile.c index 19288247c44..f013a5653bb 100644 --- a/source4/libcli/clifile.c +++ b/source4/libcli/clifile.c @@ -23,6 +23,7 @@ #include "system/filesys.h" #include "libcli/raw/libcliraw.h" #include "libcli/libcli.h" +#include "system/dir.h" /**************************************************************************** Hard/Symlink a file (UNIX extensions). @@ -148,6 +149,101 @@ NTSTATUS smbcli_unlink(struct smbcli_tree *tree, const char *fname) return smb_raw_unlink(tree, &parms); } +struct wcard_delete_state { + struct smbcli_tree *tree; + NTSTATUS status; + char *error_name; /* To help debugging. */ +}; + +static void del_fn(struct clilist_file_info *finfo, + const char *pattern, + void *priv) +{ + NTSTATUS status; + union smb_unlink parms; + char *filename = NULL; + char *dirname = NULL; + char *p = NULL; + struct wcard_delete_state *state = (struct wcard_delete_state *)priv; + + if (ISDOT(finfo->name) || ISDOTDOT(finfo->name)) { + return; + } + dirname = talloc_strdup(state, pattern); + if (dirname == NULL) { + TALLOC_FREE(state->error_name); + state->status = NT_STATUS_NO_MEMORY; + return; + } + p = strrchr_m(dirname, '\\'); + if (p != NULL) { + /* Remove the terminating '\' */ + *p = '\0'; + } + if (dirname[0] != '\0') { + filename = talloc_asprintf(dirname, + "%s\\%s", + dirname, + finfo->name); + } else { + filename = talloc_asprintf(dirname, + "%s", + finfo->name); + } + if (filename == NULL) { + TALLOC_FREE(dirname); + TALLOC_FREE(state->error_name); + state->status = NT_STATUS_NO_MEMORY; + return; + } + parms.unlink.in.pattern = filename; + parms.unlink.in.attrib = FILE_ATTRIBUTE_SYSTEM | + FILE_ATTRIBUTE_HIDDEN; + status = smb_raw_unlink(state->tree, &parms); + if (NT_STATUS_IS_OK(state->status)) { + state->status = status; + if (!NT_STATUS_IS_OK(status)) { + /* + * Save off the name we failed to + * delete to help debugging. + */ + state->error_name = talloc_move(state, &filename); + } + } + TALLOC_FREE(dirname); +} + +/**************************************************************************** + Delete a file, possibly with a wildcard pattern. +****************************************************************************/ +NTSTATUS smbcli_unlink_wcard(struct smbcli_tree *tree, const char *pattern) +{ + NTSTATUS status; + int ret; + struct wcard_delete_state *state = NULL; + + if (strchr(pattern, '*') == NULL) { + /* No wildcard, just call smbcli_unlink(). */ + return smbcli_unlink(tree, pattern); + } + state = talloc_zero(tree, struct wcard_delete_state); + if (state == NULL) { + return NT_STATUS_NO_MEMORY; + } + state->tree = tree; + ret = smbcli_list(tree, + pattern, + FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN, + del_fn, + state); + status = state->status; + TALLOC_FREE(state); + if (ret < 0) { + return NT_STATUS_UNSUCCESSFUL; + } + return status; +} + /**************************************************************************** Create a directory. ****************************************************************************/ diff --git a/source4/libcli/libcli.h b/source4/libcli/libcli.h index 652a9f5d182..9d2a3240483 100644 --- a/source4/libcli/libcli.h +++ b/source4/libcli/libcli.h @@ -158,6 +158,11 @@ NTSTATUS smbcli_rename(struct smbcli_tree *tree, const char *fname_src, ****************************************************************************/ NTSTATUS smbcli_unlink(struct smbcli_tree *tree, const char *fname); +/**************************************************************************** + Delete a wildcard pattern of files. +****************************************************************************/ +NTSTATUS smbcli_unlink_wcard(struct smbcli_tree *tree, const char *fname); + /**************************************************************************** Create a directory. ****************************************************************************/ -- 2.25.1 From cf661f306afaf66feeea11bb2d9e7f7e3c988914 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 12:10:14 -0800 Subject: [PATCH 09/99] CVE-2021-44141: s4: libcli: In smbcli_deltree() use smbcli_unlink_wcard() in place of smbcli_unlink(). We know we have a wildcard mask here. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/libcli/clideltree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/libcli/clideltree.c b/source4/libcli/clideltree.c index 01b33313213..3e4f9fb834f 100644 --- a/source4/libcli/clideltree.c +++ b/source4/libcli/clideltree.c @@ -119,7 +119,7 @@ int smbcli_deltree(struct smbcli_tree *tree, const char *dname) if (asprintf(&mask, "%s\\*", dname) < 0) { return -1; } - smbcli_unlink(dstate.tree, mask); + smbcli_unlink_wcard(dstate.tree, mask); smbcli_list(dstate.tree, mask, FILE_ATTRIBUTE_DIRECTORY|FILE_ATTRIBUTE_HIDDEN|FILE_ATTRIBUTE_SYSTEM, delete_fn, &dstate); -- 2.25.1 From a0fd6cd62f3d773371fa5f460306a562e524e6eb Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 12:08:49 -0800 Subject: [PATCH 10/99] CVE-2021-44141: s4: torture: In raw.notify test use smbcli_unlink_wcard() in place of smbcli_unlink(). We know we have a wildcard mask here. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/torture/raw/notify.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/torture/raw/notify.c b/source4/torture/raw/notify.c index 881ad6b71cc..f3c38068780 100644 --- a/source4/torture/raw/notify.c +++ b/source4/torture/raw/notify.c @@ -254,7 +254,7 @@ static bool test_notify_dir(struct torture_context *tctx, torture_comment(tctx, "Testing notify on wildcard unlink for %d files\n", count-1); /* (2nd unlink) do a wildcard unlink */ - status = smbcli_unlink(cli2->tree, BASEDIR_CN1_DIR "\\test*.txt"); + status = smbcli_unlink_wcard(cli2->tree, BASEDIR_CN1_DIR "\\test*.txt"); torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb_raw_changenotify_recv"); -- 2.25.1 From 6f9580493e250a00d100a3f96253d00bd4294b55 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 17:52:37 -0800 Subject: [PATCH 11/99] CVE-2021-44141: s4: torture: Use smbcli_unlink_wcard() to remove wildcards in base.chkpath test. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/torture/basic/base.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source4/torture/basic/base.c b/source4/torture/basic/base.c index e91fef1d6c2..232ba9c5cb3 100644 --- a/source4/torture/basic/base.c +++ b/source4/torture/basic/base.c @@ -1461,7 +1461,7 @@ static bool torture_chkpath_test(struct torture_context *tctx, /* cleanup from an old run */ smbcli_rmdir(cli->tree, "\\chkpath.dir\\dir2"); - smbcli_unlink(cli->tree, "\\chkpath.dir\\*"); + smbcli_unlink_wcard(cli->tree, "\\chkpath.dir\\*"); smbcli_rmdir(cli->tree, "\\chkpath.dir"); if (NT_STATUS_IS_ERR(smbcli_mkdir(cli->tree, "\\chkpath.dir"))) { @@ -1516,7 +1516,7 @@ static bool torture_chkpath_test(struct torture_context *tctx, } smbcli_rmdir(cli->tree, "\\chkpath.dir\\dir2"); - smbcli_unlink(cli->tree, "\\chkpath.dir\\*"); + smbcli_unlink_wcard(cli->tree, "\\chkpath.dir\\*"); smbcli_rmdir(cli->tree, "\\chkpath.dir"); return ret; -- 2.25.1 From 745d08fe10a824ef1fe1fcf57fadfd6c8b6ae216 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 17:58:58 -0800 Subject: [PATCH 12/99] CVE-2021-44141: s4: torture: Use smbcli_unlink_wcard() to cleanup in base.mangle test. Avoid using smbcli_unlink() calls with wildcard names. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/torture/basic/mangle_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/torture/basic/mangle_test.c b/source4/torture/basic/mangle_test.c index 0c3ccd54bf9..9bd3cf55dfb 100644 --- a/source4/torture/basic/mangle_test.c +++ b/source4/torture/basic/mangle_test.c @@ -195,7 +195,7 @@ bool torture_mangle(struct torture_context *torture, } } - smbcli_unlink(cli->tree, "\\mangle_test\\*"); + smbcli_unlink_wcard(cli->tree, "\\mangle_test\\*"); if (NT_STATUS_IS_ERR(smbcli_rmdir(cli->tree, "\\mangle_test"))) { printf("ERROR: Failed to remove directory\n"); return false; -- 2.25.1 From ee3a5f2ee00cbd78446cff2e815f6cf3600e17a8 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 18:03:57 -0800 Subject: [PATCH 13/99] CVE-2021-44141: s4: torture: Use smbcli_unlink_wcard() in base.casetable test. Avoid smbcli_unlink() calls with a wildcard path. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/torture/basic/utable.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/torture/basic/utable.c b/source4/torture/basic/utable.c index da2fe2e0b37..a3ddf1a7621 100644 --- a/source4/torture/basic/utable.c +++ b/source4/torture/basic/utable.c @@ -195,7 +195,7 @@ bool torture_casetable(struct torture_context *tctx, smbcli_close(cli->tree, fnum); } - smbcli_unlink(cli->tree, "\\utable\\*"); + smbcli_unlink_wcard(cli->tree, "\\utable\\*"); smbcli_rmdir(cli->tree, "\\utable"); return true; -- 2.25.1 From 2cfbfd3e0a6fd012944cad4e0fe7fe2a8688b7cc Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 18:08:32 -0800 Subject: [PATCH 14/99] CVE-2021-44141: s4: torture: Use smbcli_unlink_wcard() to setup and cleanup in masktest. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/torture/masktest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/torture/masktest.c b/source4/torture/masktest.c index 5b2457a92e9..e311769a43d 100644 --- a/source4/torture/masktest.c +++ b/source4/torture/masktest.c @@ -225,7 +225,7 @@ static void test_mask(int argc, char *argv[], smbcli_mkdir(cli->tree, "\\masktest"); - smbcli_unlink(cli->tree, "\\masktest\\*"); + smbcli_unlink_wcard(cli->tree, "\\masktest\\*"); if (argc >= 2) { while (argc >= 2) { -- 2.25.1 From db095ee5f039dc079200f1791c62c168ec57f2aa Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 14:23:10 -0800 Subject: [PATCH 15/99] CVE-2021-44141: s4: libcli: smbcli_unlink() is no longer used with wildcard patterns. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/libcli/clifile.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/source4/libcli/clifile.c b/source4/libcli/clifile.c index f013a5653bb..c6a5cd5a5e7 100644 --- a/source4/libcli/clifile.c +++ b/source4/libcli/clifile.c @@ -140,11 +140,7 @@ NTSTATUS smbcli_unlink(struct smbcli_tree *tree, const char *fname) union smb_unlink parms; parms.unlink.in.pattern = fname; - if (strchr(fname, '*')) { - parms.unlink.in.attrib = FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN; - } else { - parms.unlink.in.attrib = FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_DIRECTORY; - } + parms.unlink.in.attrib = FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_DIRECTORY; return smb_raw_unlink(tree, &parms); } -- 2.25.1 From 57fbf7564c7a0bea68ec80f774deb2fba22f3afb Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 13:47:07 -0800 Subject: [PATCH 16/99] CVE-2021-44141: s3: torture: Add torture_deltree() for setup and teardown. Not yet used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/proto.h | 1 + source3/torture/torture.c | 127 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+) diff --git a/source3/torture/proto.h b/source3/torture/proto.h index 65fa17523d8..d4db60f9dde 100644 --- a/source3/torture/proto.h +++ b/source3/torture/proto.h @@ -74,6 +74,7 @@ bool torture_ioctl_test(int dummy); bool torture_chkpath_test(int dummy); NTSTATUS torture_setup_unix_extensions(struct cli_state *cli); void torture_conn_set_sockopt(struct cli_state *cli); +void torture_deltree(struct cli_state *cli, const char *dname); /* The following definitions come from torture/utable.c */ diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 4a886614ae1..a995b54df9c 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -458,6 +458,133 @@ void torture_conn_set_sockopt(struct cli_state *cli) smbXcli_conn_set_sockopt(cli->conn, sockops); } +static NTSTATUS torture_delete_fn(struct file_info *finfo, + const char *pattern, + void *state) +{ + NTSTATUS status; + char *filename = NULL; + char *dirname = NULL; + char *p = NULL; + TALLOC_CTX *frame = talloc_stackframe(); + struct cli_state *cli = (struct cli_state *)state; + + if (ISDOT(finfo->name) || ISDOTDOT(finfo->name)) { + TALLOC_FREE(frame); + return NT_STATUS_OK; + } + + dirname = talloc_strdup(frame, pattern); + if (dirname == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + p = strrchr_m(dirname, '\\'); + if (p != NULL) { + /* Remove the terminating '\' */ + *p = '\0'; + } + if (dirname[0] != '\0') { + filename = talloc_asprintf(frame, + "%s\\%s", + dirname, + finfo->name); + } else { + filename = talloc_asprintf(frame, + "%s", + finfo->name); + } + if (filename == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + if (finfo->attr & FILE_ATTRIBUTE_DIRECTORY) { + char *subdirname = talloc_asprintf(frame, + "%s\\*", + filename); + if (subdirname == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + status = cli_list(cli, + subdirname, + FILE_ATTRIBUTE_DIRECTORY | + FILE_ATTRIBUTE_HIDDEN | + FILE_ATTRIBUTE_SYSTEM, + torture_delete_fn, + cli); + if (NT_STATUS_IS_OK(status)) { + printf("torture_delete_fn: cli_list " + "of %s failed (%s)\n", + subdirname, + nt_errstr(status)); + TALLOC_FREE(frame); + return status; + } + status = cli_rmdir(cli, filename); + } else { + status = cli_unlink(cli, + filename, + FILE_ATTRIBUTE_SYSTEM | + FILE_ATTRIBUTE_HIDDEN); + } + if (!NT_STATUS_IS_OK(status)) { + if (finfo->attr & FILE_ATTRIBUTE_DIRECTORY) { + printf("torture_delete_fn: cli_rmdir" + " of %s failed (%s)\n", + filename, + nt_errstr(status)); + } else { + printf("torture_delete_fn: cli_unlink" + " of %s failed (%s)\n", + filename, + nt_errstr(status)); + } + } + TALLOC_FREE(frame); + return status; +} + +void torture_deltree(struct cli_state *cli, const char *dname) +{ + char *mask = NULL; + NTSTATUS status; + + /* It might be a file */ + (void)cli_unlink(cli, + dname, + FILE_ATTRIBUTE_SYSTEM | + FILE_ATTRIBUTE_HIDDEN); + + mask = talloc_asprintf(cli, + "%s\\*", + dname); + if (mask == NULL) { + printf("torture_deltree: talloc_asprintf failed\n"); + return; + } + + status = cli_list(cli, + mask, + FILE_ATTRIBUTE_DIRECTORY | + FILE_ATTRIBUTE_HIDDEN| + FILE_ATTRIBUTE_SYSTEM, + torture_delete_fn, + cli); + if (!NT_STATUS_IS_OK(status)) { + printf("torture_deltree: cli_list of %s failed (%s)\n", + mask, + nt_errstr(status)); + } + TALLOC_FREE(mask); + status = cli_rmdir(cli, dname); + if (!NT_STATUS_IS_OK(status)) { + printf("torture_deltree: cli_rmdir of %s failed (%s)\n", + dname, + nt_errstr(status)); + } +} + /* check if the server produced the expected dos or nt error code */ static bool check_both_error(int line, NTSTATUS status, uint8_t eclass, uint32_t ecode, NTSTATUS nterr) -- 2.25.1 From 74fe15a05ad913112c4a76431b9e280e17ab2ee4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 12:51:54 -0800 Subject: [PATCH 17/99] CVE-2021-44141: s3: torture: In run_smb1_wild_mangle_unlink_test() use torture_deltree() for setup and cleanup. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/torture.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index a995b54df9c..cb11524446a 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -12814,10 +12814,7 @@ static bool run_smb1_wild_mangle_unlink_test(int dummy) } /* Start fresh. */ - cli_unlink(cli, - star_name, - FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_rmdir(cli, dname); + torture_deltree(cli, dname); /* * Create two files - 'a' and '*'. @@ -12927,10 +12924,7 @@ static bool run_smb1_wild_mangle_unlink_test(int dummy) TALLOC_FREE(mangled_name); if (cli != NULL) { - cli_unlink(cli, - star_name, - FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_rmdir(cli, dname); + torture_deltree(cli, dname); torture_close_connection(cli); } -- 2.25.1 From 919b3c8d3fb9c5248a79ff01e096353cab3fd9f0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 13:51:12 -0800 Subject: [PATCH 18/99] CVE-2021-44141: s3: torture: In run_smb1_wild_mangle_rename_test() use torture_deltree() for setup and cleanup. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/torture.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index cb11524446a..5b5b5287d55 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -12972,10 +12972,7 @@ static bool run_smb1_wild_mangle_rename_test(int dummy) smbXcli_conn_set_sockopt(cli->conn, sockops); /* Ensure we start from fresh. */ - cli_unlink(cli, - wild_name, - FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_posix_rmdir(cli_posix, dname); + torture_deltree(cli, dname); /* * Create two files - 'foo' and 'fo*'. @@ -13094,13 +13091,10 @@ static bool run_smb1_wild_mangle_rename_test(int dummy) TALLOC_FREE(windows_rename_src); if (cli != NULL) { - cli_unlink(cli, - wild_name, - FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); + torture_deltree(cli, dname); torture_close_connection(cli); } - cli_posix_rmdir(cli_posix, dname); torture_close_connection(cli_posix); return correct; -- 2.25.1 From 04304b9f92cb6b17fa368ed039f84f3a75cf9016 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 14:13:41 -0800 Subject: [PATCH 19/99] CVE-2021-44141: s3: torture: In torture_utable(), use torture_deltree() for setup. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/utable.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/torture/utable.c b/source3/torture/utable.c index a912c4cb3b0..01f6b2e51e9 100644 --- a/source3/torture/utable.c +++ b/source3/torture/utable.c @@ -43,8 +43,8 @@ bool torture_utable(int dummy) memset(valid, 0, sizeof(valid)); + torture_deltree(cli, "\\utable"); cli_mkdir(cli, "\\utable"); - cli_unlink(cli, "\\utable\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); for (c=1; c < 0x10000; c++) { size_t size = 0; -- 2.25.1 From 18ac36f7aed4e46f2c029d67c0e97ce4c32e9bbe Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 14:14:53 -0800 Subject: [PATCH 20/99] CVE-2021-44141: s3: torture: In torture_casetable(), use torture_deltree() for setup and cleanup. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/utable.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/source3/torture/utable.c b/source3/torture/utable.c index 01f6b2e51e9..059cae9edd1 100644 --- a/source3/torture/utable.c +++ b/source3/torture/utable.c @@ -146,8 +146,7 @@ bool torture_casetable(int dummy) memset(equiv, 0, sizeof(equiv)); - cli_unlink(cli, "\\utable\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_rmdir(cli, "\\utable"); + torture_deltree(cli, "\\utable"); if (!NT_STATUS_IS_OK(cli_mkdir(cli, "\\utable"))) { printf("Failed to create utable directory!\n"); return False; @@ -205,8 +204,7 @@ bool torture_casetable(int dummy) cli_close(cli, fnum); } - cli_unlink(cli, "\\utable\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_rmdir(cli, "\\utable"); + torture_deltree(cli, "\\utable"); return True; } -- 2.25.1 From ff64b0f32d0f0b41926badff7aab53c32759bc94 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 14:16:38 -0800 Subject: [PATCH 21/99] CVE-2021-44141: s3: torture: In torture_chkpath_test(), use torture_deltree() for setup and cleanup. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/torture.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 5b5b5287d55..e4343cf3050 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -9933,9 +9933,7 @@ bool torture_chkpath_test(int dummy) printf("starting chkpath test\n"); /* cleanup from an old run */ - cli_rmdir(cli, "\\chkpath.dir\\dir2"); - cli_unlink(cli, "\\chkpath.dir\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_rmdir(cli, "\\chkpath.dir"); + torture_deltree(cli, "\\chkpath.dir"); status = cli_mkdir(cli, "\\chkpath.dir"); if (!NT_STATUS_IS_OK(status)) { @@ -9996,9 +9994,7 @@ bool torture_chkpath_test(int dummy) ret = False; } - cli_rmdir(cli, "\\chkpath.dir\\dir2"); - cli_unlink(cli, "\\chkpath.dir\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_rmdir(cli, "\\chkpath.dir"); + torture_deltree(cli, "\\chkpath.dir"); if (!torture_close_connection(cli)) { return False; -- 2.25.1 From 8349c57f76fcd53810620fc79d6b892ed2141ae6 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 14:18:56 -0800 Subject: [PATCH 22/99] CVE-2021-44141: s3: torture: In run_streamerror(), use torture_deltree() for setup. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/torture.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index e4343cf3050..9ad0f2120e5 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -12464,8 +12464,7 @@ static bool run_streamerror(int dummy) return false; } - cli_unlink(cli, "\\testdir_streamerror\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_rmdir(cli, dname); + torture_deltree(cli, dname); status = cli_mkdir(cli, dname); if (!NT_STATUS_IS_OK(status)) { -- 2.25.1 From cf109e26b7ae918833cb4610dbcba8dc9f7c5bc0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 14:20:07 -0800 Subject: [PATCH 23/99] CVE-2021-44141: s3: torture: In test_mask(), use torture_deltree() for setup. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/masktest.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source3/torture/masktest.c b/source3/torture/masktest.c index ebd156167cd..d7485cfb5eb 100644 --- a/source3/torture/masktest.c +++ b/source3/torture/masktest.c @@ -355,10 +355,9 @@ static void test_mask(int argc, char *argv[], int fc_len = strlen(filechars); TALLOC_CTX *ctx = talloc_tos(); + torture_deltree(cli, "\\masktest"); cli_mkdir(cli, "\\masktest"); - cli_unlink(cli, "\\masktest\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - if (argc >= 2) { while (argc >= 2) { mask = talloc_asprintf(ctx, -- 2.25.1 From c249f1d09d60fb637a118c0ade7714cc3fcb1866 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 14:21:47 -0800 Subject: [PATCH 24/99] CVE-2021-44141: s3: torture: In torture_mangle(), use torture_deltree() for setup and cleanup. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/mangle_test.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/source3/torture/mangle_test.c b/source3/torture/mangle_test.c index 5832a92cdda..92754b9eeb6 100644 --- a/source3/torture/mangle_test.c +++ b/source3/torture/mangle_test.c @@ -188,8 +188,7 @@ bool torture_mangle(int dummy) return False; } - cli_unlink(cli, "\\mangle_test\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_rmdir(cli, "\\mangle_test"); + torture_deltree(cli, "\\mangle_test"); if (!NT_STATUS_IS_OK(cli_mkdir(cli, "\\mangle_test"))) { printf("ERROR: Failed to make directory\n"); @@ -212,11 +211,7 @@ bool torture_mangle(int dummy) } } - cli_unlink(cli, "\\mangle_test\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - if (!NT_STATUS_IS_OK(cli_rmdir(cli, "\\mangle_test"))) { - printf("ERROR: Failed to remove directory\n"); - return False; - } + torture_deltree(cli, "\\mangle_test"); printf("\nTotal collisions %u/%u - %.2f%% (%u failures)\n", collisions, total, (100.0*collisions) / total, failures); -- 2.25.1 From 6c40cda03e7011fe6ad7df2170a11bcfcce38e40 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 14:10:41 -0800 Subject: [PATCH 25/99] CVE-2021-44141: s3: torture: In run_smb1_wild_mangle_unlink_test() use a valid pathname for rename target. The server will not be supporting wildcard rename soon. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/torture.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 9ad0f2120e5..b80a1113a5f 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -12941,7 +12941,7 @@ static bool run_smb1_wild_mangle_rename_test(int dummy) const char *foostar_name = "smb1_wild_mangle_rename/fo*"; const char *wild_name = "smb1_wild_mangle_rename/*"; char *windows_rename_src = NULL; - const char *windows_rename_dst = "smb1_wild_mangle_rename\\ba*"; + const char *windows_rename_dst = "smb1_wild_mangle_rename\\bar"; char *mangled_name = NULL; NTSTATUS status; -- 2.25.1 From b39ba559c078b371a3b212a4c26e589fae811417 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 12:05:20 -0800 Subject: [PATCH 26/99] CVE-2021-44141: s4: torture: Remove the wildcard unlink test code. This is pre WindowXP SMB1 functionality, and we need to remove this from the server in order to move towards SMB2-only, so the test must go. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/torture/raw/unlink.c | 72 ------------------------------------ 1 file changed, 72 deletions(-) diff --git a/source4/torture/raw/unlink.c b/source4/torture/raw/unlink.c index 4f198fa2759..53059aa576a 100644 --- a/source4/torture/raw/unlink.c +++ b/source4/torture/raw/unlink.c @@ -112,78 +112,6 @@ static bool test_unlink(struct torture_context *tctx, struct smbcli_state *cli) status = smb_raw_unlink(cli->tree, &io); CHECK_STATUS(status, NT_STATUS_FILE_IS_A_DIRECTORY); - printf("Trying wildcards\n"); - smbcli_close(cli->tree, smbcli_open(cli->tree, fname, O_RDWR|O_CREAT, DENY_NONE)); - io.unlink.in.pattern = BASEDIR "\\t*.t"; - io.unlink.in.attrib = 0; - status = smb_raw_unlink(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_NO_SUCH_FILE); - - io.unlink.in.pattern = BASEDIR "\\z*"; - io.unlink.in.attrib = 0; - status = smb_raw_unlink(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_NO_SUCH_FILE); - - io.unlink.in.pattern = BASEDIR "\\z*"; - io.unlink.in.attrib = FILE_ATTRIBUTE_DIRECTORY; - status = smb_raw_unlink(cli->tree, &io); - - if (torture_setting_bool(tctx, "samba3", false)) { - /* - * In Samba3 we gave up upon getting the error codes in - * wildcard unlink correct. Trying gentest showed that this is - * irregular beyond our capabilities. So for - * FILE_ATTRIBUTE_DIRECTORY we always return NAME_INVALID. - * Tried by jra and vl. If others feel like solving this - * puzzle, please tell us :-) - */ - CHECK_STATUS(status, NT_STATUS_OBJECT_NAME_INVALID); - } - else { - CHECK_STATUS(status, NT_STATUS_NO_SUCH_FILE); - } - - io.unlink.in.pattern = BASEDIR "\\*"; - io.unlink.in.attrib = FILE_ATTRIBUTE_DIRECTORY; - status = smb_raw_unlink(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_OBJECT_NAME_INVALID); - - io.unlink.in.pattern = BASEDIR "\\?"; - io.unlink.in.attrib = FILE_ATTRIBUTE_DIRECTORY; - status = smb_raw_unlink(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_OBJECT_NAME_INVALID); - - io.unlink.in.pattern = BASEDIR "\\t*"; - io.unlink.in.attrib = FILE_ATTRIBUTE_DIRECTORY; - status = smb_raw_unlink(cli->tree, &io); - if (torture_setting_bool(tctx, "samba3", false)) { - CHECK_STATUS(status, NT_STATUS_OBJECT_NAME_INVALID); - } - else { - CHECK_STATUS(status, NT_STATUS_OK); - } - - smbcli_close(cli->tree, smbcli_open(cli->tree, fname, O_RDWR|O_CREAT, DENY_NONE)); - - io.unlink.in.pattern = BASEDIR "\\*.dat"; - io.unlink.in.attrib = FILE_ATTRIBUTE_DIRECTORY; - status = smb_raw_unlink(cli->tree, &io); - if (torture_setting_bool(tctx, "samba3", false)) { - CHECK_STATUS(status, NT_STATUS_OBJECT_NAME_INVALID); - } - else { - CHECK_STATUS(status, NT_STATUS_NO_SUCH_FILE); - } - - io.unlink.in.pattern = BASEDIR "\\*.tx?"; - io.unlink.in.attrib = 0; - status = smb_raw_unlink(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_OK); - - status = smb_raw_unlink(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_NO_SUCH_FILE); - - done: smb_raw_exit(cli->session); smbcli_deltree(cli->tree, BASEDIR); -- 2.25.1 From 05d2d29964ef5ef0e519b9621002138423d99bf9 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 13:22:39 -0800 Subject: [PATCH 27/99] CVE-2021-44141: s4: torture: Remove the wildcard rename test code. This is pre WindowXP SMB1 functionality, and we need to remove this from the server in order to move towards SMB2-only, so the test must go. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/torture/raw/rename.c | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/source4/torture/raw/rename.c b/source4/torture/raw/rename.c index 63e13c0546f..5f48c055984 100644 --- a/source4/torture/raw/rename.c +++ b/source4/torture/raw/rename.c @@ -146,39 +146,6 @@ static bool test_mv(struct torture_context *tctx, status = smb_raw_rename(cli->tree, &io); CHECK_STATUS(status, NT_STATUS_OBJECT_NAME_NOT_FOUND); - - torture_comment(tctx, "trying wildcard rename\n"); - io.rename.in.pattern1 = BASEDIR "\\*.txt"; - io.rename.in.pattern2 = fname1; - - status = smb_raw_rename(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_OK); - - torture_comment(tctx, "and again\n"); - status = smb_raw_rename(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_OK); - - torture_comment(tctx, "Trying extension change\n"); - io.rename.in.pattern1 = BASEDIR "\\*.txt"; - io.rename.in.pattern2 = BASEDIR "\\*.bak"; - status = smb_raw_rename(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_OK); - - status = smb_raw_rename(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_NO_SUCH_FILE); - - torture_comment(tctx, "Checking attrib handling\n"); - torture_set_file_attribute(cli->tree, BASEDIR "\\test1.bak", FILE_ATTRIBUTE_HIDDEN); - io.rename.in.pattern1 = BASEDIR "\\test1.bak"; - io.rename.in.pattern2 = BASEDIR "\\*.txt"; - io.rename.in.attrib = 0; - status = smb_raw_rename(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_NO_SUCH_FILE); - - io.rename.in.attrib = FILE_ATTRIBUTE_HIDDEN; - status = smb_raw_rename(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_OK); - done: smbcli_close(cli->tree, fnum); smb_raw_exit(cli->session); -- 2.25.1 From 80d8a557dda29441f4091ec76bed86d23fd3f223 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 12:18:35 -0800 Subject: [PATCH 28/99] CVE-2021-44141: s3: torture: Remove the wildcard unlink test code. This is pre WindowXP SMB1 functionality, and we need to remove this from the server in order to move towards SMB2-only, so the test must go. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- selftest/todo_smb2_tests_to_port.list | 2 - source3/selftest/tests.py | 2 +- source3/torture/torture.c | 69 --------------------------- 3 files changed, 1 insertion(+), 72 deletions(-) diff --git a/selftest/todo_smb2_tests_to_port.list b/selftest/todo_smb2_tests_to_port.list index a9d7b8b48c5..dc1df963918 100644 --- a/selftest/todo_smb2_tests_to_port.list +++ b/selftest/todo_smb2_tests_to_port.list @@ -242,7 +242,6 @@ samba3.smbtorture_s3.crypt_client.TRANS2(nt4_dc_smb1) samba3.smbtorture_s3.crypt_client.UID-REGRESSION-TEST(nt4_dc_smb1) samba3.smbtorture_s3.crypt_client.UNLINK(nt4_dc_smb1) samba3.smbtorture_s3.crypt_client.W2K(nt4_dc_smb1) -samba3.smbtorture_s3.crypt_client.WILDDELETE(nt4_dc_smb1) samba3.smbtorture_s3.crypt_client.XCOPY(nt4_dc_smb1) samba3.smbtorture_s3.crypt.POSIX-ACL-OPLOCK(nt4_dc_smb1) samba3.smbtorture_s3.crypt.POSIX-ACL-SHAREROOT(nt4_dc_smb1) @@ -327,7 +326,6 @@ samba3.smbtorture_s3.plain.TRANS2(fileserver_smb1) samba3.smbtorture_s3.plain.UID-REGRESSION-TEST(fileserver_smb1) samba3.smbtorture_s3.plain.UNLINK(fileserver_smb1) samba3.smbtorture_s3.plain.W2K(fileserver_smb1) -samba3.smbtorture_s3.plain.WILDDELETE(fileserver_smb1) samba3.smbtorture_s3.plain.WINDOWS-BAD-SYMLINK(nt4_dc_smb1) samba3.smbtorture_s3.plain.XCOPY(fileserver_smb1) samba3.smbtorture_s3.vfs_aio_fork(fileserver_smb1).RW1(fileserver_smb1) diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 0654d8b0495..4751e4437aa 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -113,7 +113,7 @@ fileserver_tests = [ "UNLINK", "BROWSE", "ATTR", "TRANS2", "TORTURE", "OPLOCK1", "OPLOCK2", "OPLOCK4", "STREAMERROR", "DIR", "DIR1", "DIR-CREATETIME", "TCON", "TCONDEV", "RW1", "RW2", "RW3", "LARGE_READX", "RW-SIGNING", - "OPEN", "XCOPY", "RENAME", "DELETE", "DELETE-LN", "WILDDELETE", "PROPERTIES", "W2K", + "OPEN", "XCOPY", "RENAME", "DELETE", "DELETE-LN", "PROPERTIES", "W2K", "TCON2", "IOCTL", "CHKPATH", "FDSESS", "CHAIN1", "CHAIN2", "OWNER-RIGHTS", "CHAIN3", "PIDHIGH", "CLI_SPLICE", "UID-REGRESSION-TEST", "SHORTNAME-TEST", diff --git a/source3/torture/torture.c b/source3/torture/torture.c index b80a1113a5f..0d3b01326b1 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -6121,71 +6121,6 @@ static bool run_delete_print_test(int dummy) return correct; } -/* - Test wildcard delete. - */ -static bool run_wild_deletetest(int dummy) -{ - struct cli_state *cli = NULL; - const char *dname = "\\WTEST"; - const char *fname = "\\WTEST\\A"; - const char *wunlink_name = "\\WTEST\\*"; - uint16_t fnum1 = (uint16_t)-1; - bool correct = false; - NTSTATUS status; - - printf("starting wildcard delete test\n"); - - if (!torture_open_connection(&cli, 0)) { - return false; - } - - smbXcli_conn_set_sockopt(cli->conn, sockops); - - cli_unlink(cli, fname, 0); - cli_rmdir(cli, dname); - status = cli_mkdir(cli, dname); - if (!NT_STATUS_IS_OK(status)) { - printf("mkdir of %s failed %s!\n", dname, nt_errstr(status)); - goto fail; - } - status = cli_openx(cli, fname, O_CREAT|O_RDONLY, DENY_NONE, &fnum1); - if (!NT_STATUS_IS_OK(status)) { - printf("open of %s failed %s!\n", fname, nt_errstr(status)); - goto fail; - } - status = cli_close(cli, fnum1); - fnum1 = -1; - - /* - * Note the unlink attribute-type of zero. This should - * map into FILE_ATTRIBUTE_NORMAL at the server even - * on a wildcard delete. - */ - - status = cli_unlink(cli, wunlink_name, 0); - if (!NT_STATUS_IS_OK(status)) { - printf("unlink of %s failed %s!\n", - wunlink_name, nt_errstr(status)); - goto fail; - } - - printf("finished wildcard delete test\n"); - - correct = true; - - fail: - - if (fnum1 != (uint16_t)-1) cli_close(cli, fnum1); - cli_unlink(cli, fname, 0); - cli_rmdir(cli, dname); - - if (cli && !torture_close_connection(cli)) { - correct = false; - } - return correct; -} - static bool run_deletetest_ln(int dummy) { struct cli_state *cli; @@ -15148,10 +15083,6 @@ static struct { .name = "DELETE-PRINT", .fn = run_delete_print_test, }, - { - .name = "WILDDELETE", - .fn = run_wild_deletetest, - }, { .name = "DELETE-LN", .fn = run_deletetest_ln, -- 2.25.1 From d57802650f48391ef906283963c86e31466702c4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 12:24:07 -0800 Subject: [PATCH 29/99] CVE-2021-44141: s3: smbd: Remove support for SMBcopy SMB_COM_COPY (0x29) It's not used in our client code or tested. From MS-CIFS. This command was introduced in the LAN Manager 1.0 dialect It was rendered obsolete in the NT LAN Manager dialect. This command was used to perform server-side file copies, but is no longer used. Clients SHOULD NOT send requests using this command code. Servers receiving requests with this command code SHOULD return STATUS_NOT_IMPLEMENTED (ERRDOS/ERRbadfunc). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 416 ++----------------------------------------- 1 file changed, 11 insertions(+), 405 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index f85d1122a07..faef4b23a3c 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -8766,416 +8766,22 @@ NTSTATUS copy_file(TALLOC_CTX *ctx, /**************************************************************************** Reply to a file copy. + + From MS-CIFS. + + This command was introduced in the LAN Manager 1.0 dialect + It was rendered obsolete in the NT LAN Manager dialect. + This command was used to perform server-side file copies, but + is no longer used. Clients SHOULD + NOT send requests using this command code. + Servers receiving requests with this command code + SHOULD return STATUS_NOT_IMPLEMENTED (ERRDOS/ERRbadfunc). ****************************************************************************/ void reply_copy(struct smb_request *req) { - connection_struct *conn = req->conn; - struct smb_filename *smb_fname_src = NULL; - struct smb_filename *smb_fname_src_dir = NULL; - struct smb_filename *smb_fname_dst = NULL; - char *fname_src = NULL; - char *fname_dst = NULL; - char *fname_src_mask = NULL; - char *fname_src_dir = NULL; - const char *p; - int count=0; - int error = ERRnoaccess; - int tid2; - int ofun; - int flags; - bool target_is_directory=False; - bool source_has_wild = False; - bool dest_has_wild = False; - NTSTATUS status; - uint32_t ucf_flags_src = UCF_ALWAYS_ALLOW_WCARD_LCOMP | - ucf_flags_from_smb_request(req); - uint32_t ucf_flags_dst = UCF_ALWAYS_ALLOW_WCARD_LCOMP | - ucf_flags_from_smb_request(req); - TALLOC_CTX *ctx = talloc_tos(); - START_PROFILE(SMBcopy); - - if (req->wct < 3) { - reply_nterror(req, NT_STATUS_INVALID_PARAMETER); - goto out; - } - - tid2 = SVAL(req->vwv+0, 0); - ofun = SVAL(req->vwv+1, 0); - flags = SVAL(req->vwv+2, 0); - - p = (const char *)req->buf; - p += srvstr_get_path_req(ctx, req, &fname_src, p, STR_TERMINATE, - &status); - if (!NT_STATUS_IS_OK(status)) { - reply_nterror(req, status); - goto out; - } - p += srvstr_get_path_req(ctx, req, &fname_dst, p, STR_TERMINATE, - &status); - if (!NT_STATUS_IS_OK(status)) { - reply_nterror(req, status); - goto out; - } - - DEBUG(3,("reply_copy : %s -> %s\n", fname_src, fname_dst)); - - if (tid2 != conn->cnum) { - /* can't currently handle inter share copies XXXX */ - DEBUG(3,("Rejecting inter-share copy\n")); - reply_nterror(req, NT_STATUS_BAD_DEVICE_TYPE); - goto out; - } - - status = filename_convert(ctx, conn, - fname_src, - ucf_flags_src, - 0, - &smb_fname_src); - if (!NT_STATUS_IS_OK(status)) { - if (NT_STATUS_EQUAL(status,NT_STATUS_PATH_NOT_COVERED)) { - reply_botherror(req, NT_STATUS_PATH_NOT_COVERED, - ERRSRV, ERRbadpath); - goto out; - } - reply_nterror(req, status); - goto out; - } - - status = filename_convert(ctx, conn, - fname_dst, - ucf_flags_dst, - 0, - &smb_fname_dst); - if (!NT_STATUS_IS_OK(status)) { - if (NT_STATUS_EQUAL(status,NT_STATUS_PATH_NOT_COVERED)) { - reply_botherror(req, NT_STATUS_PATH_NOT_COVERED, - ERRSRV, ERRbadpath); - goto out; - } - reply_nterror(req, status); - goto out; - } - - target_is_directory = VALID_STAT_OF_DIR(smb_fname_dst->st); - - if ((flags&1) && target_is_directory) { - reply_nterror(req, NT_STATUS_NO_SUCH_FILE); - goto out; - } - - if ((flags&2) && !target_is_directory) { - reply_nterror(req, NT_STATUS_OBJECT_PATH_NOT_FOUND); - goto out; - } - - if ((flags&(1<<5)) && VALID_STAT_OF_DIR(smb_fname_src->st)) { - /* wants a tree copy! XXXX */ - DEBUG(3,("Rejecting tree copy\n")); - reply_nterror(req, NT_STATUS_INVALID_PARAMETER); - goto out; - } - - /* Split up the directory from the filename/mask. */ - status = split_fname_dir_mask(ctx, smb_fname_src->base_name, - &fname_src_dir, &fname_src_mask); - if (!NT_STATUS_IS_OK(status)) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - - if (!req->posix_pathnames) { - char *orig_src_lcomp = NULL; - char *orig_dst_lcomp = NULL; - /* - * Check the wildcard mask *before* - * unmangling. As mangling is done - * for names that can't be returned - * to Windows the unmangled name may - * contain Windows wildcard characters. - */ - orig_src_lcomp = get_original_lcomp(ctx, - conn, - fname_src, - ucf_flags_src); - if (orig_src_lcomp == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - orig_dst_lcomp = get_original_lcomp(ctx, - conn, - fname_dst, - ucf_flags_dst); - if (orig_dst_lcomp == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - source_has_wild = ms_has_wild(orig_src_lcomp); - dest_has_wild = ms_has_wild(orig_dst_lcomp); - TALLOC_FREE(orig_src_lcomp); - TALLOC_FREE(orig_dst_lcomp); - } - - /* - * We should only check the mangled cache - * here if unix_convert failed. This means - * that the path in 'mask' doesn't exist - * on the file system and so we need to look - * for a possible mangle. This patch from - * Tine Smukavec . - */ - if (!VALID_STAT(smb_fname_src->st) && - mangle_is_mangled(fname_src_mask, conn->params)) { - char *new_mask = NULL; - mangle_lookup_name_from_8_3(ctx, fname_src_mask, - &new_mask, conn->params); - - /* Use demangled name if one was successfully found. */ - if (new_mask) { - TALLOC_FREE(fname_src_mask); - fname_src_mask = new_mask; - } - } - - if (!source_has_wild) { - - /* - * Only one file needs to be copied. Append the mask back onto - * the directory. - */ - TALLOC_FREE(smb_fname_src->base_name); - if (ISDOT(fname_src_dir)) { - /* Ensure we use canonical names on open. */ - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s", - fname_src_mask); - } else { - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s/%s", - fname_src_dir, - fname_src_mask); - } - if (!smb_fname_src->base_name) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - - if (dest_has_wild) { - char *fname_dst_mod = NULL; - if (!resolve_wildcards(smb_fname_dst, - smb_fname_src->base_name, - smb_fname_dst->base_name, - &fname_dst_mod)) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - TALLOC_FREE(smb_fname_dst->base_name); - smb_fname_dst->base_name = fname_dst_mod; - } - - status = check_name(conn, smb_fname_src); - if (!NT_STATUS_IS_OK(status)) { - reply_nterror(req, status); - goto out; - } - - status = check_name(conn, smb_fname_dst); - if (!NT_STATUS_IS_OK(status)) { - reply_nterror(req, status); - goto out; - } - - status = copy_file(ctx, conn, smb_fname_src, smb_fname_dst, - ofun, count, target_is_directory); - - if(!NT_STATUS_IS_OK(status)) { - reply_nterror(req, status); - goto out; - } else { - count++; - } - } else { - struct smb_Dir *dir_hnd = NULL; - const char *dname = NULL; - char *talloced = NULL; - long offset = 0; - - /* - * There is a wildcard that requires us to actually read the - * src dir and copy each file matching the mask to the dst. - * Right now streams won't be copied, but this could - * presumably be added with a nested loop for reach dir entry. - */ - SMB_ASSERT(!smb_fname_src->stream_name); - SMB_ASSERT(!smb_fname_dst->stream_name); - - smb_fname_src->stream_name = NULL; - smb_fname_dst->stream_name = NULL; - - if (strequal(fname_src_mask,"????????.???")) { - TALLOC_FREE(fname_src_mask); - fname_src_mask = talloc_strdup(ctx, "*"); - if (!fname_src_mask) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - } - - smb_fname_src_dir = synthetic_smb_fname(talloc_tos(), - fname_src_dir, - NULL, - NULL, - smb_fname_src->twrp, - smb_fname_src->flags); - if (smb_fname_src_dir == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - - status = check_name(conn, smb_fname_src_dir); - if (!NT_STATUS_IS_OK(status)) { - reply_nterror(req, status); - goto out; - } - - dir_hnd = OpenDir(ctx, - conn, - smb_fname_src_dir, - fname_src_mask, - 0); - if (dir_hnd == NULL) { - status = map_nt_error_from_unix(errno); - reply_nterror(req, status); - goto out; - } - - error = ERRbadfile; - - /* Iterate over the src dir copying each entry to the dst. */ - while ((dname = ReadDirName(dir_hnd, &offset, - &smb_fname_src->st, &talloced))) { - char *destname = NULL; - - if (ISDOT(dname) || ISDOTDOT(dname)) { - TALLOC_FREE(talloced); - continue; - } - - if (IS_VETO_PATH(conn, dname)) { - TALLOC_FREE(talloced); - continue; - } - - if(!mask_match(dname, fname_src_mask, - conn->case_sensitive)) { - TALLOC_FREE(talloced); - continue; - } - - error = ERRnoaccess; - - /* Get the src smb_fname struct setup. */ - TALLOC_FREE(smb_fname_src->base_name); - if (ISDOT(fname_src_dir)) { - /* Ensure we use canonical names on open. */ - smb_fname_src->base_name = - talloc_asprintf(smb_fname_src, "%s", - dname); - } else { - smb_fname_src->base_name = - talloc_asprintf(smb_fname_src, "%s/%s", - fname_src_dir, dname); - } - - if (!smb_fname_src->base_name) { - TALLOC_FREE(dir_hnd); - TALLOC_FREE(talloced); - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - - if (!resolve_wildcards(ctx, smb_fname_src->base_name, - smb_fname_dst->base_name, - &destname)) { - TALLOC_FREE(talloced); - continue; - } - if (!destname) { - TALLOC_FREE(dir_hnd); - TALLOC_FREE(talloced); - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - - TALLOC_FREE(smb_fname_dst->base_name); - smb_fname_dst->base_name = destname; - - ZERO_STRUCT(smb_fname_src->st); - vfs_stat(conn, smb_fname_src); - - status = openat_pathref_fsp(conn->cwd_fsp, - smb_fname_src); - if (!NT_STATUS_IS_OK(status)) { - DBG_INFO("openat_pathref_fsp [%s] failed: %s\n", - smb_fname_str_dbg(smb_fname_src), - nt_errstr(status)); - break; - } - - if (!is_visible_fsp(smb_fname_src->fsp)) { - TALLOC_FREE(talloced); - continue; - } - - status = check_name(conn, smb_fname_src); - if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(dir_hnd); - TALLOC_FREE(talloced); - reply_nterror(req, status); - goto out; - } - - status = check_name(conn, smb_fname_dst); - if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(dir_hnd); - TALLOC_FREE(talloced); - reply_nterror(req, status); - goto out; - } - - DEBUG(3,("reply_copy : doing copy on %s -> %s\n", - smb_fname_src->base_name, - smb_fname_dst->base_name)); - - status = copy_file(ctx, conn, smb_fname_src, - smb_fname_dst, ofun, count, - target_is_directory); - if (NT_STATUS_IS_OK(status)) { - count++; - } - - TALLOC_FREE(talloced); - } - TALLOC_FREE(dir_hnd); - } - - if (count == 0) { - reply_nterror(req, dos_to_ntstatus(ERRDOS, error)); - goto out; - } - - reply_outbuf(req, 1, 0); - SSVAL(req->outbuf,smb_vwv0,count); - out: - TALLOC_FREE(smb_fname_src); - TALLOC_FREE(smb_fname_src_dir); - TALLOC_FREE(smb_fname_dst); - TALLOC_FREE(fname_src); - TALLOC_FREE(fname_dst); - TALLOC_FREE(fname_src_mask); - TALLOC_FREE(fname_src_dir); - + reply_nterror(req, NT_STATUS_NOT_IMPLEMENTED); END_PROFILE(SMBcopy); return; } -- 2.25.1 From 79ae11f3cb464ffe381c62451ad38125f1872156 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 12:31:44 -0800 Subject: [PATCH 30/99] CVE-2021-44141: s3: smbd: In reply_unlink() remove the possibility of receiving a wildcard name. This was the only user of "has_wild=true" passed to unlink_internals(). Next commit will remove this functionality from unlink_internals(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index faef4b23a3c..dd8c536ffac 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3537,10 +3537,8 @@ void reply_unlink(struct smb_request *req) struct smb_filename *smb_fname = NULL; uint32_t dirtype; NTSTATUS status; - uint32_t ucf_flags = UCF_ALWAYS_ALLOW_WCARD_LCOMP | - ucf_flags_from_smb_request(req); + uint32_t ucf_flags = ucf_flags_from_smb_request(req); TALLOC_CTX *ctx = talloc_tos(); - bool has_wild = false; START_PROFILE(SMBunlink); @@ -3573,22 +3571,9 @@ void reply_unlink(struct smb_request *req) goto out; } - if (!req->posix_pathnames) { - char *lcomp = get_original_lcomp(ctx, - conn, - name, - ucf_flags); - if (lcomp == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - has_wild = ms_has_wild(lcomp); - TALLOC_FREE(lcomp); - } - DEBUG(3,("reply_unlink : %s\n", smb_fname_str_dbg(smb_fname))); - status = unlink_internals(conn, req, dirtype, smb_fname, has_wild); + status = unlink_internals(conn, req, dirtype, smb_fname, false); if (!NT_STATUS_IS_OK(status)) { if (open_was_deferred(req->xconn, req->mid)) { /* We have re-scheduled this call. */ -- 2.25.1 From e4c3d31854fa1969d946bd6a7cfcf25db5d21d7b Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 12:53:29 -0800 Subject: [PATCH 31/99] CVE-2021-44141: s3: smbd: Change unlink_internals() to ignore has_wild parameter. It's always passed as false now so we can remove the (horrible) enumeration code for unlink. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 226 +++++-------------------------------------- 1 file changed, 26 insertions(+), 200 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index dd8c536ffac..abdf5f01fd8 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3283,11 +3283,9 @@ NTSTATUS unlink_internals(connection_struct *conn, { char *fname_dir = NULL; char *fname_mask = NULL; - int count=0; NTSTATUS status = NT_STATUS_OK; struct smb_filename *smb_fname_dir = NULL; TALLOC_CTX *ctx = talloc_tos(); - int ret; /* Split up the directory from the filename/mask. */ status = split_fname_dir_mask(ctx, smb_fname->base_name, @@ -3316,209 +3314,37 @@ NTSTATUS unlink_internals(connection_struct *conn, } } - if (!has_wild) { - - /* - * Only one file needs to be unlinked. Append the mask back - * onto the directory. - */ - TALLOC_FREE(smb_fname->base_name); - if (ISDOT(fname_dir)) { - /* Ensure we use canonical names on open. */ - smb_fname->base_name = talloc_asprintf(smb_fname, - "%s", - fname_mask); - } else { - smb_fname->base_name = talloc_asprintf(smb_fname, - "%s/%s", - fname_dir, - fname_mask); - } - if (!smb_fname->base_name) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - if (dirtype == 0) { - dirtype = FILE_ATTRIBUTE_NORMAL; - } - - status = check_name(conn, smb_fname); - if (!NT_STATUS_IS_OK(status)) { - goto out; - } - - status = do_unlink(conn, req, smb_fname, dirtype); - if (!NT_STATUS_IS_OK(status)) { - goto out; - } - - count++; + /* + * Only one file needs to be unlinked. Append the mask back + * onto the directory. + */ + TALLOC_FREE(smb_fname->base_name); + if (ISDOT(fname_dir)) { + /* Ensure we use canonical names on open. */ + smb_fname->base_name = talloc_asprintf(smb_fname, + "%s", + fname_mask); } else { - struct smb_Dir *dir_hnd = NULL; - long offset = 0; - const char *dname = NULL; - char *talloced = NULL; - - if ((dirtype & SAMBA_ATTRIBUTES_MASK) == FILE_ATTRIBUTE_DIRECTORY) { - status = NT_STATUS_OBJECT_NAME_INVALID; - goto out; - } - if (dirtype == 0) { - dirtype = FILE_ATTRIBUTE_NORMAL; - } - - if (strequal(fname_mask,"????????.???")) { - TALLOC_FREE(fname_mask); - fname_mask = talloc_strdup(ctx, "*"); - if (!fname_mask) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - } - - smb_fname_dir = synthetic_smb_fname(talloc_tos(), - fname_dir, - NULL, - NULL, - smb_fname->twrp, - smb_fname->flags); - if (smb_fname_dir == NULL) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - status = check_name(conn, smb_fname_dir); - if (!NT_STATUS_IS_OK(status)) { - goto out; - } - - dir_hnd = OpenDir(talloc_tos(), conn, smb_fname_dir, fname_mask, - dirtype); - if (dir_hnd == NULL) { - status = map_nt_error_from_unix(errno); - goto out; - } - - /* XXXX the CIFS spec says that if bit0 of the flags2 field is set then - the pattern matches against the long name, otherwise the short name - We don't implement this yet XXXX - */ - - status = NT_STATUS_NO_SUCH_FILE; - - while ((dname = ReadDirName(dir_hnd, &offset, - &smb_fname->st, &talloced))) { - TALLOC_CTX *frame = talloc_stackframe(); - char *p = NULL; - struct smb_filename *f = NULL; - - /* Quick check for "." and ".." */ - if (ISDOT(dname) || ISDOTDOT(dname)) { - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - continue; - } - - if (IS_VETO_PATH(conn, dname)) { - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - continue; - } - - if(!mask_match(dname, fname_mask, - conn->case_sensitive)) { - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - continue; - } - - if (ISDOT(fname_dir)) { - /* Ensure we use canonical names on open. */ - p = talloc_asprintf(smb_fname, "%s", dname); - } else { - p = talloc_asprintf(smb_fname, "%s/%s", - fname_dir, dname); - } - if (p == NULL) { - TALLOC_FREE(dir_hnd); - status = NT_STATUS_NO_MEMORY; - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - goto out; - } - f = synthetic_smb_fname(frame, - p, - NULL, - &smb_fname->st, - smb_fname->twrp, - smb_fname->flags); - if (f == NULL) { - TALLOC_FREE(dir_hnd); - status = NT_STATUS_NO_MEMORY; - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - goto out; - } - - ret = vfs_stat(conn, f); - if (ret != 0) { - status = map_nt_error_from_unix(errno); - TALLOC_FREE(dir_hnd); - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - goto out; - } - - status = openat_pathref_fsp(conn->cwd_fsp, f); - if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) && - (f->flags & SMB_FILENAME_POSIX_PATH) && - S_ISLNK(f->st.st_ex_mode)) - { - status = NT_STATUS_OK; - } - if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(dir_hnd); - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - goto out; - } - - if (!is_visible_fsp(f->fsp)) { - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - continue; - } - - status = check_name(conn, f); - if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(dir_hnd); - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - goto out; - } - - status = do_unlink(conn, req, f, dirtype); - if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(dir_hnd); - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - goto out; - } - - count++; - DBG_DEBUG("successful unlink [%s]\n", - smb_fname_str_dbg(f)); - - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - } - TALLOC_FREE(dir_hnd); + smb_fname->base_name = talloc_asprintf(smb_fname, + "%s/%s", + fname_dir, + fname_mask); + } + if (!smb_fname->base_name) { + status = NT_STATUS_NO_MEMORY; + goto out; + } + if (dirtype == 0) { + dirtype = FILE_ATTRIBUTE_NORMAL; } - if (count == 0 && NT_STATUS_IS_OK(status) && errno != 0) { - status = map_nt_error_from_unix(errno); + status = check_name(conn, smb_fname); + if (!NT_STATUS_IS_OK(status)) { + goto out; } + status = do_unlink(conn, req, smb_fname, dirtype); + out: TALLOC_FREE(smb_fname_dir); TALLOC_FREE(fname_dir); -- 2.25.1 From 945c9264243c811346d28ce0aeb740bdbe1083dc Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 13:03:03 -0800 Subject: [PATCH 32/99] CVE-2021-44141: s3: smbd: Remove 'bool has_wild' parameter from unlink_internals(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/printing/nt_printing.c | 2 +- source3/smbd/proto.h | 3 +-- source3/smbd/reply.c | 5 ++--- source3/smbd/trans2.c | 3 +-- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/source3/printing/nt_printing.c b/source3/printing/nt_printing.c index b172ed92c6e..a47afda4a84 100644 --- a/source3/printing/nt_printing.c +++ b/source3/printing/nt_printing.c @@ -2042,7 +2042,7 @@ static NTSTATUS driver_unlink_internals(connection_struct *conn, goto err_out; } - status = unlink_internals(conn, NULL, 0, smb_fname, false); + status = unlink_internals(conn, NULL, 0, smb_fname); err_out: talloc_free(tmp_ctx); return status; diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index bf7401f5191..9454d44968b 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -995,8 +995,7 @@ void reply_ctemp(struct smb_request *req); NTSTATUS unlink_internals(connection_struct *conn, struct smb_request *req, uint32_t dirtype, - struct smb_filename *smb_fname, - bool has_wcard); + struct smb_filename *smb_fname); void reply_unlink(struct smb_request *req); ssize_t fake_sendfile(struct smbXsrv_connection *xconn, files_struct *fsp, off_t startpos, size_t nread); diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index abdf5f01fd8..e996b809243 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3278,8 +3278,7 @@ static NTSTATUS do_unlink(connection_struct *conn, NTSTATUS unlink_internals(connection_struct *conn, struct smb_request *req, uint32_t dirtype, - struct smb_filename *smb_fname, - bool has_wild) + struct smb_filename *smb_fname) { char *fname_dir = NULL; char *fname_mask = NULL; @@ -3399,7 +3398,7 @@ void reply_unlink(struct smb_request *req) DEBUG(3,("reply_unlink : %s\n", smb_fname_str_dbg(smb_fname))); - status = unlink_internals(conn, req, dirtype, smb_fname, false); + status = unlink_internals(conn, req, dirtype, smb_fname); if (!NT_STATUS_IS_OK(status)) { if (open_was_deferred(req->xconn, req->mid)) { /* We have re-scheduled this call. */ diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 4f6d92955cf..c455b68725e 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -6514,8 +6514,7 @@ NTSTATUS hardlink_internals(TALLOC_CTX *ctx, status = unlink_internals(conn, req, FILE_ATTRIBUTE_NORMAL, - smb_fname_new, - false); + smb_fname_new); if (!NT_STATUS_IS_OK(status)) { goto out; } -- 2.25.1 From 410126c7fb939c3bebee318376602e5084a93e12 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 13:56:31 -0800 Subject: [PATCH 33/99] CVE-2021-44141: s3: smbd: Remove UCF_ALWAYS_ALLOW_WCARD_LCOMP flag from pathname processing in reply_mv(). We are no longer supporting wildcard rename via SMBmv (0x7) as WindowsXP SMB1 and above do not use it. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index e996b809243..0bc3db4c9ca 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -8242,10 +8242,8 @@ void reply_mv(struct smb_request *req) const char *src_original_lcomp = NULL; struct smb_filename *smb_fname_dst = NULL; const char *dst_original_lcomp = NULL; - uint32_t src_ucf_flags = ucf_flags_from_smb_request(req) | - (!req->posix_pathnames ? UCF_ALWAYS_ALLOW_WCARD_LCOMP : 0); - uint32_t dst_ucf_flags = ucf_flags_from_smb_request(req) | - (!req->posix_pathnames ? UCF_ALWAYS_ALLOW_WCARD_LCOMP : 0); + uint32_t src_ucf_flags = ucf_flags_from_smb_request(req); + uint32_t dst_ucf_flags = ucf_flags_from_smb_request(req); bool stream_rename = false; START_PROFILE(SMBmv); -- 2.25.1 From 7b0eba7ff03aa79400c4dbac224122209c8e8995 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:07:07 -0800 Subject: [PATCH 34/99] CVE-2021-44141: s3: smbd: In smb_file_rename_information() (SMB_FILE_RENAME_INFORMATION info level) prevent destination wildcards. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/trans2.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index c455b68725e..1eac9b0d2db 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -7438,8 +7438,7 @@ static NTSTATUS smb_file_rename_information(connection_struct *conn, * the newname instead. */ char *base_name = NULL; - uint32_t ucf_flags = UCF_ALWAYS_ALLOW_WCARD_LCOMP| - ucf_flags_from_smb_request(req); + uint32_t ucf_flags = ucf_flags_from_smb_request(req); /* newname must *not* be a stream name. */ if (newname[0] == ':') { -- 2.25.1 From 07b47529426d4623270c2541b840bf71cfca9d59 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:08:13 -0800 Subject: [PATCH 35/99] CVE-2021-44141: s3: smbd: In SMBntrename (0xa5) prevent wildcards in destination name. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/nttrans.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c index ffff822e221..5e6163f0433 100644 --- a/source3/smbd/nttrans.c +++ b/source3/smbd/nttrans.c @@ -1771,6 +1771,11 @@ void reply_ntrename(struct smb_request *req) goto out; } + if (!req->posix_pathnames && ms_has_wild(newname)) { + reply_nterror(req, NT_STATUS_OBJECT_PATH_SYNTAX_BAD); + goto out; + } + if (!req->posix_pathnames) { /* The newname must begin with a ':' if the oldname contains a ':'. */ -- 2.25.1 From 9d0c2fd42fc77de7a9748ef2ecd7d284b5105e37 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:12:46 -0800 Subject: [PATCH 36/99] CVE-2021-44141: s3: smbd: In reply_ntrename() remove the UCF_ALWAYS_ALLOW_WCARD_LCOMP flag for destination lookups. We know the destination will never be a wildcard. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/nttrans.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c index 5e6163f0433..e25a59a7cb9 100644 --- a/source3/smbd/nttrans.c +++ b/source3/smbd/nttrans.c @@ -1788,14 +1788,6 @@ void reply_ntrename(struct smb_request *req) } } - /* - * If this is a rename operation, allow wildcards and save the - * destination's last component. - */ - if (rename_type == RENAME_FLAG_RENAME) { - ucf_flags_dst |= UCF_ALWAYS_ALLOW_WCARD_LCOMP; - } - /* rename_internals() calls unix_convert(), so don't call it here. */ status = filename_convert(ctx, conn, oldname, -- 2.25.1 From c7678425514417b524800e65098bc8608849c457 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:14:57 -0800 Subject: [PATCH 37/99] CVE-2021-44141: s3: smbd: In reply_ntrename(), never set dest_has_wcard. It can never be true. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/nttrans.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c index e25a59a7cb9..f5cb82024c8 100644 --- a/source3/smbd/nttrans.c +++ b/source3/smbd/nttrans.c @@ -1816,10 +1816,6 @@ void reply_ntrename(struct smb_request *req) goto out; } - if (!req->posix_pathnames) { - dest_has_wcard = ms_has_wild(dst_original_lcomp); - } - status = filename_convert(ctx, conn, newname, ucf_flags_dst, -- 2.25.1 From 992864a49f099052c452eb5e1da733f39294d94a Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:17:51 -0800 Subject: [PATCH 38/99] CVE-2021-44141: s3: smbd: In reply_ntrename() remove 'bool dest_has_wcard' and all uses. It's always false now. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/nttrans.c | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c index f5cb82024c8..b0f55ade267 100644 --- a/source3/smbd/nttrans.c +++ b/source3/smbd/nttrans.c @@ -1732,7 +1732,6 @@ void reply_ntrename(struct smb_request *req) const char *dst_original_lcomp = NULL; const char *p; NTSTATUS status; - bool dest_has_wcard = False; uint32_t attrs; uint32_t ucf_flags_src = ucf_flags_from_smb_request(req); uint32_t ucf_flags_dst = ucf_flags_from_smb_request(req); @@ -1862,27 +1861,20 @@ void reply_ntrename(struct smb_request *req) DELETE_ACCESS); break; case RENAME_FLAG_HARD_LINK: - if (dest_has_wcard) { - /* No wildcards. */ - status = NT_STATUS_OBJECT_PATH_SYNTAX_BAD; - } else { - status = hardlink_internals(ctx, conn, - req, - false, - smb_fname_old, - smb_fname_new); - } + status = hardlink_internals(ctx, + conn, + req, + false, + smb_fname_old, + smb_fname_new); break; case RENAME_FLAG_COPY: - if (dest_has_wcard) { - /* No wildcards. */ - status = NT_STATUS_OBJECT_PATH_SYNTAX_BAD; - } else { - status = copy_internals(ctx, conn, req, - smb_fname_old, - smb_fname_new, - attrs); - } + status = copy_internals(ctx, + conn, + req, + smb_fname_old, + smb_fname_new, + attrs); break; case RENAME_FLAG_MOVE_CLUSTER_INFORMATION: status = NT_STATUS_INVALID_PARAMETER; -- 2.25.1 From 848b891d978d928dc3199f7f1e146bfc0b7ab988 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:25:03 -0800 Subject: [PATCH 39/99] CVE-2021-44141: s3: smbd: Prepare to remove wildcard matching from rename_internals(). src_has_wild and dest_has_wild can never be true. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 0bc3db4c9ca..3eaadb33861 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7860,20 +7860,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, goto out; } - if (!(smb_fname_src->flags & SMB_FILENAME_POSIX_PATH)) { - /* - * Check the wildcard mask *before* - * unmangling. As mangling is done - * for names that can't be returned - * to Windows the unmangled name may - * contain Windows wildcard characters. - */ - if (src_original_lcomp != NULL) { - src_has_wild = ms_has_wild(src_original_lcomp); - } - dest_has_wild = ms_has_wild(dst_original_lcomp); - } - /* * We should only check the mangled cache * here if unix_convert failed. This means -- 2.25.1 From ece00d51a7b28ec96c0a173ca40feb61aaf5dbe3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:26:28 -0800 Subject: [PATCH 40/99] CVE-2021-44141: s3: smbd: Remove dest_has_wild and all associated code from rename_internals() BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 3eaadb33861..21f40cf123e 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7841,7 +7841,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, struct smb2_create_blobs *posx = NULL; int rc; bool src_has_wild = false; - bool dest_has_wild = false; /* * Split the old name into directory and last component @@ -7923,24 +7922,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, smb_fname_str_dbg(smb_fname_dst), dst_original_lcomp)); - /* The dest name still may have wildcards. */ - if (dest_has_wild) { - char *fname_dst_mod = NULL; - if (!resolve_wildcards(smb_fname_dst, - smb_fname_src->base_name, - smb_fname_dst->base_name, - &fname_dst_mod)) { - DEBUG(6, ("rename_internals: resolve_wildcards " - "%s %s failed\n", - smb_fname_src->base_name, - smb_fname_dst->base_name)); - status = NT_STATUS_NO_MEMORY; - goto out; - } - TALLOC_FREE(smb_fname_dst->base_name); - smb_fname_dst->base_name = fname_dst_mod; - } - ZERO_STRUCT(smb_fname_src->st); rc = vfs_stat(conn, smb_fname_src); -- 2.25.1 From cafca2b7a0eca282d24d8f3571bc14ef725f30e4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:29:43 -0800 Subject: [PATCH 41/99] CVE-2021-44141: s3: smbd: Remove all wildcard code from rename_internals(). We no longer use resolve_wildcards() so comment it out for later removal. Keep the '{ ... }' block around the singleton rename for now, to keep the diff small. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 201 +------------------------------------------ 1 file changed, 4 insertions(+), 197 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 21f40cf123e..a0e61546323 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7059,6 +7059,7 @@ void reply_rmdir(struct smb_request *req) return; } +#if 0 /******************************************************************* Resolve wildcards in a filename rename. ********************************************************************/ @@ -7185,6 +7186,7 @@ static bool resolve_wildcards(TALLOC_CTX *ctx, return True; } +#endif /**************************************************************************** Ensure open files have their names updated. Updated to notify other smbd's @@ -7831,24 +7833,18 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, char *fname_src_dir = NULL; struct smb_filename *smb_fname_src_dir = NULL; char *fname_src_mask = NULL; - int count=0; NTSTATUS status = NT_STATUS_OK; - struct smb_Dir *dir_hnd = NULL; - const char *dname = NULL; char *talloced = NULL; - long offset = 0; int create_options = 0; struct smb2_create_blobs *posx = NULL; int rc; - bool src_has_wild = false; /* * Split the old name into directory and last component * strings. Note that unix_convert may have stripped off a * leading ./ from both name and newname if the rename is * at the root of the share. We need to make sure either both - * name and newname contain a / character or neither of them do - * as this is checked in resolve_wildcards(). + * name and newname contain a / character or neither of them do. */ /* Split up the directory from the filename/mask. */ @@ -7888,7 +7884,7 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, } } - if (!src_has_wild) { + { files_struct *fsp; /* @@ -7992,195 +7988,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, nt_errstr(status), smb_fname_str_dbg(smb_fname_src), smb_fname_str_dbg(smb_fname_dst))); - goto out; - } - - /* - * Wildcards - process each file that matches. - */ - if (strequal(fname_src_mask, "????????.???")) { - TALLOC_FREE(fname_src_mask); - fname_src_mask = talloc_strdup(ctx, "*"); - if (!fname_src_mask) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - } - - smb_fname_src_dir = synthetic_smb_fname(talloc_tos(), - fname_src_dir, - NULL, - NULL, - smb_fname_src->twrp, - smb_fname_src->flags); - if (smb_fname_src_dir == NULL) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - status = check_name(conn, smb_fname_src_dir); - if (!NT_STATUS_IS_OK(status)) { - goto out; - } - - dir_hnd = OpenDir(talloc_tos(), conn, smb_fname_src_dir, fname_src_mask, - attrs); - if (dir_hnd == NULL) { - status = map_nt_error_from_unix(errno); - goto out; - } - - status = NT_STATUS_NO_SUCH_FILE; - /* - * Was status = NT_STATUS_OBJECT_NAME_NOT_FOUND; - * - gentest fix. JRA - */ - - while ((dname = ReadDirName(dir_hnd, &offset, &smb_fname_src->st, - &talloced))) { - files_struct *fsp = NULL; - char *destname = NULL; - bool sysdir_entry = False; - - /* Quick check for "." and ".." */ - if (ISDOT(dname) || ISDOTDOT(dname)) { - if (attrs & FILE_ATTRIBUTE_DIRECTORY) { - sysdir_entry = True; - } else { - TALLOC_FREE(talloced); - continue; - } - } - - if(!mask_match(dname, fname_src_mask, conn->case_sensitive)) { - TALLOC_FREE(talloced); - continue; - } - - if (sysdir_entry) { - status = NT_STATUS_OBJECT_NAME_INVALID; - break; - } - - TALLOC_FREE(smb_fname_src->base_name); - if (ISDOT(fname_src_dir)) { - /* Ensure we use canonical names on open. */ - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s", - dname); - } else { - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s/%s", - fname_src_dir, - dname); - } - if (!smb_fname_src->base_name) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - if (!resolve_wildcards(ctx, smb_fname_src->base_name, - smb_fname_dst->base_name, - &destname)) { - DEBUG(6, ("resolve_wildcards %s %s failed\n", - smb_fname_src->base_name, destname)); - TALLOC_FREE(talloced); - continue; - } - if (!destname) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - TALLOC_FREE(smb_fname_dst->base_name); - smb_fname_dst->base_name = destname; - - ZERO_STRUCT(smb_fname_src->st); - vfs_stat(conn, smb_fname_src); - - status = openat_pathref_fsp(conn->cwd_fsp, smb_fname_src); - if (!NT_STATUS_IS_OK(status)) { - DBG_INFO("openat_pathref_fsp [%s] failed: %s\n", - smb_fname_str_dbg(smb_fname_src), - nt_errstr(status)); - break; - } - - if (!is_visible_fsp(smb_fname_src->fsp)) { - TALLOC_FREE(talloced); - continue; - } - - create_options = 0; - - if (S_ISDIR(smb_fname_src->st.st_ex_mode)) { - create_options |= FILE_DIRECTORY_FILE; - } - - status = SMB_VFS_CREATE_FILE( - conn, /* conn */ - req, /* req */ - smb_fname_src, /* fname */ - access_mask, /* access_mask */ - (FILE_SHARE_READ | /* share_access */ - FILE_SHARE_WRITE), - FILE_OPEN, /* create_disposition*/ - create_options, /* create_options */ - 0, /* file_attributes */ - 0, /* oplock_request */ - NULL, /* lease */ - 0, /* allocation_size */ - 0, /* private_flags */ - NULL, /* sd */ - NULL, /* ea_list */ - &fsp, /* result */ - NULL, /* pinfo */ - posx, /* in_context_blobs */ - NULL); /* out_context_blobs */ - - if (!NT_STATUS_IS_OK(status)) { - DEBUG(3,("rename_internals: SMB_VFS_CREATE_FILE " - "returned %s rename %s -> %s\n", - nt_errstr(status), - smb_fname_str_dbg(smb_fname_src), - smb_fname_str_dbg(smb_fname_dst))); - break; - } - - dst_original_lcomp = talloc_strdup(smb_fname_dst, dname); - if (dst_original_lcomp == NULL) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - status = rename_internals_fsp(conn, - fsp, - smb_fname_dst, - dst_original_lcomp, - attrs, - replace_if_exists); - - close_file(req, fsp, NORMAL_CLOSE); - - if (!NT_STATUS_IS_OK(status)) { - DEBUG(3, ("rename_internals_fsp returned %s for " - "rename %s -> %s\n", nt_errstr(status), - smb_fname_str_dbg(smb_fname_src), - smb_fname_str_dbg(smb_fname_dst))); - break; - } - - count++; - - DEBUG(3,("rename_internals: doing rename on %s -> " - "%s\n", smb_fname_str_dbg(smb_fname_src), - smb_fname_str_dbg(smb_fname_src))); - TALLOC_FREE(talloced); - } - TALLOC_FREE(dir_hnd); - - if (count == 0 && NT_STATUS_IS_OK(status) && errno != 0) { - status = map_nt_error_from_unix(errno); } out: -- 2.25.1 From be70e606c61cd2da8f27718f5a227728494793e3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:31:36 -0800 Subject: [PATCH 42/99] CVE-2021-44141: s3: smbd: Remove the commented out resolve_wildcards(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 129 ------------------------------------------- 1 file changed, 129 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index a0e61546323..aec0892c81a 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7059,135 +7059,6 @@ void reply_rmdir(struct smb_request *req) return; } -#if 0 -/******************************************************************* - Resolve wildcards in a filename rename. -********************************************************************/ - -static bool resolve_wildcards(TALLOC_CTX *ctx, - const char *name1, - const char *name2, - char **pp_newname) -{ - char *name2_copy = NULL; - char *root1 = NULL; - char *root2 = NULL; - char *ext1 = NULL; - char *ext2 = NULL; - char *p,*p2, *pname1, *pname2; - - name2_copy = talloc_strdup(ctx, name2); - if (!name2_copy) { - return False; - } - - pname1 = strrchr_m(name1,'/'); - pname2 = strrchr_m(name2_copy,'/'); - - if (!pname1 || !pname2) { - return False; - } - - /* Truncate the copy of name2 at the last '/' */ - *pname2 = '\0'; - - /* Now go past the '/' */ - pname1++; - pname2++; - - root1 = talloc_strdup(ctx, pname1); - root2 = talloc_strdup(ctx, pname2); - - if (!root1 || !root2) { - return False; - } - - p = strrchr_m(root1,'.'); - if (p) { - *p = 0; - ext1 = talloc_strdup(ctx, p+1); - } else { - ext1 = talloc_strdup(ctx, ""); - } - p = strrchr_m(root2,'.'); - if (p) { - *p = 0; - ext2 = talloc_strdup(ctx, p+1); - } else { - ext2 = talloc_strdup(ctx, ""); - } - - if (!ext1 || !ext2) { - return False; - } - - p = root1; - p2 = root2; - while (*p2) { - if (*p2 == '?') { - /* Hmmm. Should this be mb-aware ? */ - *p2 = *p; - p2++; - } else if (*p2 == '*') { - *p2 = '\0'; - root2 = talloc_asprintf(ctx, "%s%s", - root2, - p); - if (!root2) { - return False; - } - break; - } else { - p2++; - } - if (*p) { - p++; - } - } - - p = ext1; - p2 = ext2; - while (*p2) { - if (*p2 == '?') { - /* Hmmm. Should this be mb-aware ? */ - *p2 = *p; - p2++; - } else if (*p2 == '*') { - *p2 = '\0'; - ext2 = talloc_asprintf(ctx, "%s%s", - ext2, - p); - if (!ext2) { - return False; - } - break; - } else { - p2++; - } - if (*p) { - p++; - } - } - - if (*ext2) { - *pp_newname = talloc_asprintf(ctx, "%s/%s.%s", - name2_copy, - root2, - ext2); - } else { - *pp_newname = talloc_asprintf(ctx, "%s/%s", - name2_copy, - root2); - } - - if (!*pp_newname) { - return False; - } - - return True; -} -#endif - /**************************************************************************** Ensure open files have their names updated. Updated to notify other smbd's asynchronously. -- 2.25.1 From cf2de328ea36011d9f4594bac84fce0b8db0889e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:35:54 -0800 Subject: [PATCH 43/99] CVE-2021-44141: s3: smbd: Inside rename_internals() remove '{ ... }' block around singleton rename code. Best viewed with 'git show -b' As we're touching the DEBUG() code, change it to modern DBG_NOTICE(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 132 +++++++++++++++++++++---------------------- 1 file changed, 64 insertions(+), 68 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index aec0892c81a..e9814037b1e 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7708,6 +7708,7 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, char *talloced = NULL; int create_options = 0; struct smb2_create_blobs *posx = NULL; + struct files_struct *fsp = NULL; int rc; /* @@ -7755,70 +7756,67 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, } } - { - files_struct *fsp; + /* + * Only one file needs to be renamed. Append the mask back + * onto the directory. + */ + TALLOC_FREE(smb_fname_src->base_name); + if (ISDOT(fname_src_dir)) { + /* Ensure we use canonical names on open. */ + smb_fname_src->base_name = talloc_asprintf(smb_fname_src, + "%s", + fname_src_mask); + } else { + smb_fname_src->base_name = talloc_asprintf(smb_fname_src, + "%s/%s", + fname_src_dir, + fname_src_mask); + } + if (!smb_fname_src->base_name) { + status = NT_STATUS_NO_MEMORY; + goto out; + } - /* - * Only one file needs to be renamed. Append the mask back - * onto the directory. - */ - TALLOC_FREE(smb_fname_src->base_name); - if (ISDOT(fname_src_dir)) { - /* Ensure we use canonical names on open. */ - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s", - fname_src_mask); - } else { - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s/%s", - fname_src_dir, - fname_src_mask); - } - if (!smb_fname_src->base_name) { - status = NT_STATUS_NO_MEMORY; - goto out; - } + DBG_NOTICE("case_sensitive = %d, " + "case_preserve = %d, short case preserve = %d, " + "directory = %s, newname = %s, " + "last_component_dest = %s\n", + conn->case_sensitive, conn->case_preserve, + conn->short_case_preserve, + smb_fname_str_dbg(smb_fname_src), + smb_fname_str_dbg(smb_fname_dst), + dst_original_lcomp); - DEBUG(3, ("rename_internals: case_sensitive = %d, " - "case_preserve = %d, short case preserve = %d, " - "directory = %s, newname = %s, " - "last_component_dest = %s\n", - conn->case_sensitive, conn->case_preserve, - conn->short_case_preserve, - smb_fname_str_dbg(smb_fname_src), - smb_fname_str_dbg(smb_fname_dst), - dst_original_lcomp)); + ZERO_STRUCT(smb_fname_src->st); - ZERO_STRUCT(smb_fname_src->st); + rc = vfs_stat(conn, smb_fname_src); + if (rc == -1) { + status = map_nt_error_from_unix_common(errno); + goto out; + } - rc = vfs_stat(conn, smb_fname_src); - if (rc == -1) { - status = map_nt_error_from_unix_common(errno); + status = openat_pathref_fsp(conn->cwd_fsp, smb_fname_src); + if (!NT_STATUS_IS_OK(status)) { + if (!NT_STATUS_EQUAL(status, + NT_STATUS_OBJECT_NAME_NOT_FOUND)) { goto out; } - - status = openat_pathref_fsp(conn->cwd_fsp, smb_fname_src); - if (!NT_STATUS_IS_OK(status)) { - if (!NT_STATUS_EQUAL(status, - NT_STATUS_OBJECT_NAME_NOT_FOUND)) { - goto out; - } - /* - * Possible symlink src. - */ - if (!(smb_fname_src->flags & SMB_FILENAME_POSIX_PATH)) { - goto out; - } - if (!S_ISLNK(smb_fname_src->st.st_ex_mode)) { - goto out; - } + /* + * Possible symlink src. + */ + if (!(smb_fname_src->flags & SMB_FILENAME_POSIX_PATH)) { + goto out; } - - if (S_ISDIR(smb_fname_src->st.st_ex_mode)) { - create_options |= FILE_DIRECTORY_FILE; + if (!S_ISLNK(smb_fname_src->st.st_ex_mode)) { + goto out; } + } - status = SMB_VFS_CREATE_FILE( + if (S_ISDIR(smb_fname_src->st.st_ex_mode)) { + create_options |= FILE_DIRECTORY_FILE; + } + + status = SMB_VFS_CREATE_FILE( conn, /* conn */ req, /* req */ smb_fname_src, /* fname */ @@ -7839,27 +7837,25 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, posx, /* in_context_blobs */ NULL); /* out_context_blobs */ - if (!NT_STATUS_IS_OK(status)) { - DEBUG(3, ("Could not open rename source %s: %s\n", - smb_fname_str_dbg(smb_fname_src), - nt_errstr(status))); - goto out; - } + if (!NT_STATUS_IS_OK(status)) { + DBG_NOTICE("Could not open rename source %s: %s\n", + smb_fname_str_dbg(smb_fname_src), + nt_errstr(status)); + goto out; + } - status = rename_internals_fsp(conn, + status = rename_internals_fsp(conn, fsp, smb_fname_dst, dst_original_lcomp, attrs, replace_if_exists); - close_file(req, fsp, NORMAL_CLOSE); + close_file(req, fsp, NORMAL_CLOSE); - DEBUG(3, ("rename_internals: Error %s rename %s -> %s\n", - nt_errstr(status), smb_fname_str_dbg(smb_fname_src), - smb_fname_str_dbg(smb_fname_dst))); - - } + DBG_NOTICE("Error %s rename %s -> %s\n", + nt_errstr(status), smb_fname_str_dbg(smb_fname_src), + smb_fname_str_dbg(smb_fname_dst)); out: TALLOC_FREE(posx); -- 2.25.1 From fc80b553dc6c6a17fa496e1347174ffb802c3ffb Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:39:42 -0800 Subject: [PATCH 44/99] CVE-2021-44141: s3: smbd: Remove 'const char *src_original_lcomp' parameter from rename_internals(). No longer used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/nttrans.c | 1 - source3/smbd/proto.h | 1 - source3/smbd/reply.c | 2 -- source3/smbd/trans2.c | 1 - 4 files changed, 5 deletions(-) diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c index b0f55ade267..2b742411071 100644 --- a/source3/smbd/nttrans.c +++ b/source3/smbd/nttrans.c @@ -1853,7 +1853,6 @@ void reply_ntrename(struct smb_request *req) conn, req, smb_fname_old, - NULL, smb_fname_new, dst_original_lcomp, attrs, diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 9454d44968b..fa26d58bfde 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -1041,7 +1041,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, connection_struct *conn, struct smb_request *req, struct smb_filename *smb_fname_src, - const char *src_original_lcomp, struct smb_filename *smb_fname_dst, const char *dst_original_lcomp, uint32_t attrs, diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index e9814037b1e..8fc943375b4 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7694,7 +7694,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, connection_struct *conn, struct smb_request *req, struct smb_filename *smb_fname_src, - const char *src_original_lcomp, struct smb_filename *smb_fname_dst, const char *dst_original_lcomp, uint32_t attrs, @@ -7996,7 +7995,6 @@ void reply_mv(struct smb_request *req) conn, req, smb_fname_src, - src_original_lcomp, smb_fname_dst, dst_original_lcomp, attrs, diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 1eac9b0d2db..2b2643b6325 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -7527,7 +7527,6 @@ static NTSTATUS smb_file_rename_information(connection_struct *conn, conn, req, smb_fname_src, - NULL, smb_fname_dst, dst_original_lcomp, 0, -- 2.25.1 From 8c1a9ccb546e7677dc05e98bd6aa77681e0d7510 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:40:55 -0800 Subject: [PATCH 45/99] CVE-2021-44141: s3: smbd: Remove 'const char *src_original_lcomp' from reply_mv(). No longer used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 8fc943375b4..079c8da7e3c 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7879,7 +7879,6 @@ void reply_mv(struct smb_request *req) NTSTATUS status; TALLOC_CTX *ctx = talloc_tos(); struct smb_filename *smb_fname_src = NULL; - const char *src_original_lcomp = NULL; struct smb_filename *smb_fname_dst = NULL; const char *dst_original_lcomp = NULL; uint32_t src_ucf_flags = ucf_flags_from_smb_request(req); @@ -7939,16 +7938,6 @@ void reply_mv(struct smb_request *req) goto out; } - /* Get the last component of the source for rename_internals(). */ - src_original_lcomp = get_original_lcomp(ctx, - conn, - name, - dst_ucf_flags); - if (src_original_lcomp == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - status = filename_convert(ctx, conn, newname, -- 2.25.1 From 9907c8af089a6263349566f002747d84edf926d3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:08:07 -0800 Subject: [PATCH 46/99] CVE-2021-44141: s3: smbd: Move setting of dirtype if FILE_ATTRIBUTE_NORMAL to do_unlink(). Now we don't use wildcards when calling in unlink_internals() the logic inside it serves no purpose and can be replaced with a direct call to do_unlink() (which we will rename to unlink_internals()). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 079c8da7e3c..b709dc65c47 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3142,6 +3142,10 @@ static NTSTATUS do_unlink(connection_struct *conn, int ret; struct smb2_create_blobs *posx = NULL; + if (dirtype == 0) { + dirtype = FILE_ATTRIBUTE_NORMAL; + } + DEBUG(10,("do_unlink: %s, dirtype = %d\n", smb_fname_str_dbg(smb_fname), dirtype)); @@ -3333,9 +3337,6 @@ NTSTATUS unlink_internals(connection_struct *conn, status = NT_STATUS_NO_MEMORY; goto out; } - if (dirtype == 0) { - dirtype = FILE_ATTRIBUTE_NORMAL; - } status = check_name(conn, smb_fname); if (!NT_STATUS_IS_OK(status)) { -- 2.25.1 From 9fb1d11b2edafe0b2e8fb8cfbe34e1da046b3d97 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:11:20 -0800 Subject: [PATCH 47/99] CVE-2021-44141: s3: smbd: Move to modern debug calls inside do_unlink(). We will be changing its name next. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index b709dc65c47..71d7454e58f 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3146,9 +3146,9 @@ static NTSTATUS do_unlink(connection_struct *conn, dirtype = FILE_ATTRIBUTE_NORMAL; } - DEBUG(10,("do_unlink: %s, dirtype = %d\n", + DBG_DEBUG("%s, dirtype = %d\n", smb_fname_str_dbg(smb_fname), - dirtype)); + dirtype); if (!CAN_WRITE(conn)) { return NT_STATUS_MEDIA_WRITE_PROTECTED; @@ -3248,17 +3248,17 @@ static NTSTATUS do_unlink(connection_struct *conn, TALLOC_FREE(posx); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10, ("SMB_VFS_CREATEFILE failed: %s\n", - nt_errstr(status))); + DBG_DEBUG("SMB_VFS_CREATEFILE failed: %s\n", + nt_errstr(status)); return status; } status = can_set_delete_on_close(fsp, fattr); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10, ("do_unlink can_set_delete_on_close for file %s - " + DBG_DEBUG("can_set_delete_on_close for file %s - " "(%s)\n", smb_fname_str_dbg(smb_fname), - nt_errstr(status))); + nt_errstr(status)); close_file(req, fsp, NORMAL_CLOSE); return status; } -- 2.25.1 From a88596028eac6facd644afa7eb5bf9ed34915c30 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:14:40 -0800 Subject: [PATCH 48/99] CVE-2021-44141: s3: smbd: Comment out the old unlink_internals(). Rename do_unlink() -> unlink_internals(). One parameter needs changing position. The logic inside unlink_internals() is no longer needed if it doesn't accept wildcards. filename_convert() already handles mangled names just fine, so we don't need this logic. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 71d7454e58f..01de8f0c03f 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3130,10 +3130,10 @@ static NTSTATUS can_rename(connection_struct *conn, files_struct *fsp, * unlink a file with all relevant access checks *******************************************************************/ -static NTSTATUS do_unlink(connection_struct *conn, +NTSTATUS unlink_internals(connection_struct *conn, struct smb_request *req, - struct smb_filename *smb_fname, - uint32_t dirtype) + uint32_t dirtype, + struct smb_filename *smb_fname) { uint32_t fattr; files_struct *fsp; @@ -3274,6 +3274,7 @@ static NTSTATUS do_unlink(connection_struct *conn, return close_file(req, fsp, NORMAL_CLOSE); } +#if 0 /**************************************************************************** The guts of the unlink command, split out so it may be called by the NT SMB code. @@ -3351,6 +3352,7 @@ NTSTATUS unlink_internals(connection_struct *conn, TALLOC_FREE(fname_mask); return status; } +#endif /**************************************************************************** Reply to a unlink -- 2.25.1 From fad0039acabd43eaf6853af60e5d245a2691a664 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:16:52 -0800 Subject: [PATCH 49/99] CVE-2021-44141: s3: smbd: Remove the old unlink_internals() implementation. No longer used. filename_convert() already handles mangled names just fine, so we don't need this logic. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 80 -------------------------------------------- 1 file changed, 80 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 01de8f0c03f..26049b2b62b 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3274,86 +3274,6 @@ NTSTATUS unlink_internals(connection_struct *conn, return close_file(req, fsp, NORMAL_CLOSE); } -#if 0 -/**************************************************************************** - The guts of the unlink command, split out so it may be called by the NT SMB - code. -****************************************************************************/ - -NTSTATUS unlink_internals(connection_struct *conn, - struct smb_request *req, - uint32_t dirtype, - struct smb_filename *smb_fname) -{ - char *fname_dir = NULL; - char *fname_mask = NULL; - NTSTATUS status = NT_STATUS_OK; - struct smb_filename *smb_fname_dir = NULL; - TALLOC_CTX *ctx = talloc_tos(); - - /* Split up the directory from the filename/mask. */ - status = split_fname_dir_mask(ctx, smb_fname->base_name, - &fname_dir, &fname_mask); - if (!NT_STATUS_IS_OK(status)) { - goto out; - } - - /* - * We should only check the mangled cache - * here if unix_convert failed. This means - * that the path in 'mask' doesn't exist - * on the file system and so we need to look - * for a possible mangle. This patch from - * Tine Smukavec . - */ - - if (!VALID_STAT(smb_fname->st) && - mangle_is_mangled(fname_mask, conn->params)) { - char *new_mask = NULL; - mangle_lookup_name_from_8_3(ctx, fname_mask, - &new_mask, conn->params); - if (new_mask) { - TALLOC_FREE(fname_mask); - fname_mask = new_mask; - } - } - - /* - * Only one file needs to be unlinked. Append the mask back - * onto the directory. - */ - TALLOC_FREE(smb_fname->base_name); - if (ISDOT(fname_dir)) { - /* Ensure we use canonical names on open. */ - smb_fname->base_name = talloc_asprintf(smb_fname, - "%s", - fname_mask); - } else { - smb_fname->base_name = talloc_asprintf(smb_fname, - "%s/%s", - fname_dir, - fname_mask); - } - if (!smb_fname->base_name) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - status = check_name(conn, smb_fname); - if (!NT_STATUS_IS_OK(status)) { - goto out; - } - - status = do_unlink(conn, req, smb_fname, dirtype); - - out: - TALLOC_FREE(smb_fname_dir); - TALLOC_FREE(fname_dir); - TALLOC_FREE(fname_mask); - return status; -} -#endif - /**************************************************************************** Reply to a unlink ****************************************************************************/ -- 2.25.1 From 26ecf18b426eb3f2db9d60f02ece2af5e6fa057e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:35:17 -0800 Subject: [PATCH 50/99] CVE-2021-44141: s3: smbd: Handling SMB_FILE_RENAME_INFORMATION, the destination name is a single component. No errors should be allowed from filename_convert(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/trans2.c | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 2b2643b6325..95a7cc63970 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -7477,25 +7477,8 @@ static NTSTATUS smb_file_rename_information(connection_struct *conn, 0, &smb_fname_dst); - /* If an error we expect this to be - * NT_STATUS_OBJECT_PATH_NOT_FOUND */ - if (!NT_STATUS_IS_OK(status)) { - if(!NT_STATUS_EQUAL(NT_STATUS_OBJECT_PATH_NOT_FOUND, - status)) { - goto out; - } - /* Create an smb_fname to call rename_internals_fsp() */ - smb_fname_dst = synthetic_smb_fname(ctx, - base_name, - NULL, - NULL, - smb_fname_src->twrp, - smb_fname_src->flags); - if (smb_fname_dst == NULL) { - status = NT_STATUS_NO_MEMORY; - goto out; - } + goto out; } dst_original_lcomp = get_original_lcomp(smb_fname_dst, conn, -- 2.25.1 From 838985e439df0c1b741516ff141da02ecbf5656f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:45:13 -0800 Subject: [PATCH 51/99] CVE-2021-44141: s3: smbd: In rename_internals_fsp(), remove unneeded call to check_name(). All callers have gone through filename_convert(), which has already called check_name() on the destination. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 26049b2b62b..b9f1bd6b9c2 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7185,11 +7185,6 @@ NTSTATUS rename_internals_fsp(connection_struct *conn, bool dst_exists, old_is_stream, new_is_stream; int ret; - status = check_name(conn, smb_fname_dst_in); - if (!NT_STATUS_IS_OK(status)) { - return status; - } - status = parent_dirname_compatible_open(conn, smb_fname_dst_in); if (!NT_STATUS_IS_OK(status)) { return status; -- 2.25.1 From 43a9866c46b9a82af34693e5c17c0c627169cb76 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:47:13 -0800 Subject: [PATCH 52/99] CVE-2021-44141: s3: smbd: check_name() is now static to filename.c BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 2 +- source3/smbd/proto.h | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 11022dca703..44f9aebf068 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1430,7 +1430,7 @@ static NTSTATUS check_veto_path(connection_struct *conn, a valid one for the user to access. ****************************************************************************/ -NTSTATUS check_name(connection_struct *conn, +static NTSTATUS check_name(connection_struct *conn, const struct smb_filename *smb_fname) { NTSTATUS status = check_veto_path(conn, smb_fname); diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index fa26d58bfde..e6a0d6c3d51 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -358,8 +358,6 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx, NTTIME twrp, struct smb_filename **smb_fname, uint32_t ucf_flags); -NTSTATUS check_name(connection_struct *conn, - const struct smb_filename *smb_fname); NTSTATUS canonicalize_snapshot_path(struct smb_filename *smb_fname, uint32_t ucf_flags, NTTIME twrp); -- 2.25.1 From 68ee550a0dd41e31fd6ffdd1aeda8adb3595a8cf Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:49:46 -0800 Subject: [PATCH 53/99] CVE-2021-44141: s3: smbd: In rename_internals(), remove the name spliting and re-combining code. filename_convert() handles mangled names just fine, so we don't need to split the last component and check for mangle. Now we don't take wildcard names this is not needed. This was the last caller of split_fname_dir_mask(), so ifdef it out. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 67 ++------------------------------------------ 1 file changed, 2 insertions(+), 65 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index b9f1bd6b9c2..7b0eb18d744 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -1624,6 +1624,7 @@ void reply_dskattr(struct smb_request *req) return; } +#if 0 /* * Utility function to split the filename from the directory. */ @@ -1655,6 +1656,7 @@ static NTSTATUS split_fname_dir_mask(TALLOC_CTX *ctx, const char *fname_in, *fname_mask_out = fname_mask; return NT_STATUS_OK; } +#endif /**************************************************************************** Make a dir struct. @@ -7618,52 +7620,12 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, bool replace_if_exists, uint32_t access_mask) { - char *fname_src_dir = NULL; - struct smb_filename *smb_fname_src_dir = NULL; - char *fname_src_mask = NULL; NTSTATUS status = NT_STATUS_OK; - char *talloced = NULL; int create_options = 0; struct smb2_create_blobs *posx = NULL; struct files_struct *fsp = NULL; int rc; - /* - * Split the old name into directory and last component - * strings. Note that unix_convert may have stripped off a - * leading ./ from both name and newname if the rename is - * at the root of the share. We need to make sure either both - * name and newname contain a / character or neither of them do. - */ - - /* Split up the directory from the filename/mask. */ - status = split_fname_dir_mask(ctx, smb_fname_src->base_name, - &fname_src_dir, &fname_src_mask); - if (!NT_STATUS_IS_OK(status)) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - /* - * We should only check the mangled cache - * here if unix_convert failed. This means - * that the path in 'mask' doesn't exist - * on the file system and so we need to look - * for a possible mangle. This patch from - * Tine Smukavec . - */ - - if (!VALID_STAT(smb_fname_src->st) && - mangle_is_mangled(fname_src_mask, conn->params)) { - char *new_mask = NULL; - mangle_lookup_name_from_8_3(ctx, fname_src_mask, &new_mask, - conn->params); - if (new_mask) { - TALLOC_FREE(fname_src_mask); - fname_src_mask = new_mask; - } - } - if (smb_fname_src->flags & SMB_FILENAME_POSIX_PATH) { status = make_smb2_posix_create_ctx(talloc_tos(), &posx, 0777); if (!NT_STATUS_IS_OK(status)) { @@ -7673,27 +7635,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, } } - /* - * Only one file needs to be renamed. Append the mask back - * onto the directory. - */ - TALLOC_FREE(smb_fname_src->base_name); - if (ISDOT(fname_src_dir)) { - /* Ensure we use canonical names on open. */ - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s", - fname_src_mask); - } else { - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s/%s", - fname_src_dir, - fname_src_mask); - } - if (!smb_fname_src->base_name) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - DBG_NOTICE("case_sensitive = %d, " "case_preserve = %d, short case preserve = %d, " "directory = %s, newname = %s, " @@ -7776,10 +7717,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, out: TALLOC_FREE(posx); - TALLOC_FREE(talloced); - TALLOC_FREE(smb_fname_src_dir); - TALLOC_FREE(fname_src_dir); - TALLOC_FREE(fname_src_mask); return status; } -- 2.25.1 From 0163d21c31ad978182adba73bae8f0ee48c69e53 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:51:45 -0800 Subject: [PATCH 54/99] CVE-2021-44141: s3: smbd: Remove split_fname_dir_mask(). No longer used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 7b0eb18d744..d2048856cff 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -1624,40 +1624,6 @@ void reply_dskattr(struct smb_request *req) return; } -#if 0 -/* - * Utility function to split the filename from the directory. - */ -static NTSTATUS split_fname_dir_mask(TALLOC_CTX *ctx, const char *fname_in, - char **fname_dir_out, - char **fname_mask_out) -{ - const char *p = NULL; - char *fname_dir = NULL; - char *fname_mask = NULL; - - p = strrchr_m(fname_in, '/'); - if (!p) { - fname_dir = talloc_strdup(ctx, "."); - fname_mask = talloc_strdup(ctx, fname_in); - } else { - fname_dir = talloc_strndup(ctx, fname_in, - PTR_DIFF(p, fname_in)); - fname_mask = talloc_strdup(ctx, p+1); - } - - if (!fname_dir || !fname_mask) { - TALLOC_FREE(fname_dir); - TALLOC_FREE(fname_mask); - return NT_STATUS_NO_MEMORY; - } - - *fname_dir_out = fname_dir; - *fname_mask_out = fname_mask; - return NT_STATUS_OK; -} -#endif - /**************************************************************************** Make a dir struct. ****************************************************************************/ -- 2.25.1 From 1c1c7ed99466ace89eb61d4783903b8b8a718e27 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 17:51:42 -0800 Subject: [PATCH 55/99] CVE-2021-44141: s3: smbd: In call_trans2findfirst() we don't need filename_convert_with_privilege() anymore. It was extra-paranoid code now not needed as the new VFS version of filename_convert() does the same job. There are now no remaining callers of filename_convert_with_privilege(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/trans2.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 95a7cc63970..4612221dbfe 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -2757,19 +2757,12 @@ close_if_end = %d requires_resume_key = %d backup_priv = %d level = 0x%x, max_da if (backup_priv) { become_root(); as_root = true; - ntstatus = filename_convert_with_privilege(talloc_tos(), - conn, - req, - directory, - ucf_flags, - &smb_dname); - } else { - ntstatus = filename_convert(talloc_tos(), conn, + } + ntstatus = filename_convert(talloc_tos(), conn, directory, ucf_flags, 0, &smb_dname); - } if (!NT_STATUS_IS_OK(ntstatus)) { if (NT_STATUS_EQUAL(ntstatus,NT_STATUS_PATH_NOT_COVERED)) { -- 2.25.1 From 46ec23c244bc001a5bb1105a2d1e23ebfdd78ca4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 17:55:26 -0800 Subject: [PATCH 56/99] CVE-2021-44141: s3: smbd: Remove filename_convert_with_privilege(). No longer used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 21 --------------------- source3/smbd/proto.h | 6 ------ 2 files changed, 27 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 44f9aebf068..3d0f350c3dc 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -2078,27 +2078,6 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, pp_smb_fname); } -/* - * Go through all the steps to validate a filename. - * root (privileged) version. - */ - -NTSTATUS filename_convert_with_privilege(TALLOC_CTX *ctx, - connection_struct *conn, - struct smb_request *smbreq, - const char *name_in, - uint32_t ucf_flags, - struct smb_filename **pp_smb_fname) -{ - return filename_convert_internal(ctx, - conn, - smbreq, - name_in, - ucf_flags, - 0, - pp_smb_fname); -} - /* * Build the full path from a dirfsp and dirfsp relative name */ diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index e6a0d6c3d51..58b32c6908b 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -382,12 +382,6 @@ NTSTATUS filename_convert(TALLOC_CTX *mem_ctx, uint32_t ucf_flags, NTTIME twrp, struct smb_filename **pp_smb_fname); -NTSTATUS filename_convert_with_privilege(TALLOC_CTX *mem_ctx, - connection_struct *conn, - struct smb_request *smbreq, - const char *name_in, - uint32_t ucf_flags, - struct smb_filename **pp_smb_fname); /* The following definitions come from smbd/files.c */ -- 2.25.1 From 733e66aa31da219f7bc54cd380451d380d6ca3a1 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 10:10:45 -0800 Subject: [PATCH 57/99] CVE-2021-44141: s3: smbd: In filename_convert_internal(), remove call to check_name_with_privilege(). We now always pass NULL as struct smb_request *smbreq, so this code path can never be taken. Comment out check_name_with_privilege() as it's now no longer used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 3d0f350c3dc..882f41ec400 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1452,6 +1452,7 @@ static NTSTATUS check_name(connection_struct *conn, return NT_STATUS_OK; } +#if 0 /**************************************************************************** Must be called as root. Creates the struct privilege_paths attached to the struct smb_request if this call is successful. @@ -1470,6 +1471,7 @@ static NTSTATUS check_name_with_privilege(connection_struct *conn, smb_fname, smbreq); } +#endif /**************************************************************************** Check if two filenames are equal. @@ -1999,11 +2001,8 @@ static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, TALLOC_FREE(smb_fname); return status; } - } else if (!smbreq) { - status = check_name(conn, smb_fname); } else { - status = check_name_with_privilege(conn, smbreq, - smb_fname); + status = check_name(conn, smb_fname); } if (!NT_STATUS_IS_OK(status)) { DEBUG(3,("filename_convert_internal: check_name failed " -- 2.25.1 From 3f60b452049e4c10cec414a7da8709f2ceb3f929 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 10:13:13 -0800 Subject: [PATCH 58/99] CVE-2021-44141: s3: smbd: Remove unused check_name_with_privilege(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 882f41ec400..297f48496e8 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1452,27 +1452,6 @@ static NTSTATUS check_name(connection_struct *conn, return NT_STATUS_OK; } -#if 0 -/**************************************************************************** - Must be called as root. Creates the struct privilege_paths - attached to the struct smb_request if this call is successful. -****************************************************************************/ - -static NTSTATUS check_name_with_privilege(connection_struct *conn, - struct smb_request *smbreq, - const struct smb_filename *smb_fname) -{ - NTSTATUS status = check_veto_path(conn, smb_fname); - - if (!NT_STATUS_IS_OK(status)) { - return status; - } - return check_reduced_name_with_privilege(conn, - smb_fname, - smbreq); -} -#endif - /**************************************************************************** Check if two filenames are equal. This needs to be careful about whether we are case sensitive. -- 2.25.1 From 51c024a1b029c0ca66594336d5474b8cc64c4452 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 10:14:03 -0800 Subject: [PATCH 59/99] CVE-2021-44141: s3: smbd: Remove now unused check_reduced_name_with_privilege(). We now only have one function that does this check (check_reduced_name()), used everywhere. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/proto.h | 3 - source3/smbd/vfs.c | 173 ------------------------------------------- 2 files changed, 176 deletions(-) diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 58b32c6908b..5e7bc392b87 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -1300,9 +1300,6 @@ struct smb_filename *vfs_GetWd(TALLOC_CTX *ctx, connection_struct *conn); NTSTATUS check_reduced_name(connection_struct *conn, const struct smb_filename *cwd_fname, const struct smb_filename *smb_fname); -NTSTATUS check_reduced_name_with_privilege(connection_struct *conn, - const struct smb_filename *smb_fname, - struct smb_request *smbreq); int vfs_stat(struct connection_struct *conn, struct smb_filename *smb_fname); int vfs_stat_smb_basename(struct connection_struct *conn, diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c index 34831385e09..d8b7b1283fb 100644 --- a/source3/smbd/vfs.c +++ b/source3/smbd/vfs.c @@ -1122,179 +1122,6 @@ struct smb_filename *vfs_GetWd(TALLOC_CTX *ctx, connection_struct *conn) return result; } -/******************************************************************* - Reduce a file name, removing .. elements and checking that - it is below dir in the hierarchy. This uses realpath. - This function must run as root, and will return names - and valid stat structs that can be checked on open. -********************************************************************/ - -NTSTATUS check_reduced_name_with_privilege(connection_struct *conn, - const struct smb_filename *smb_fname, - struct smb_request *smbreq) -{ - NTSTATUS status; - TALLOC_CTX *ctx = talloc_tos(); - const char *conn_rootdir; - size_t rootdir_len; - char *resolved_name = NULL; - struct smb_filename *resolved_fname = NULL; - struct smb_filename *saved_dir_fname = NULL; - struct smb_filename *smb_fname_cwd = NULL; - int ret; - struct smb_filename *parent_name = NULL; - struct smb_filename *file_name = NULL; - - DEBUG(3,("check_reduced_name_with_privilege [%s] [%s]\n", - smb_fname->base_name, - conn->connectpath)); - - status = SMB_VFS_PARENT_PATHNAME(conn, - ctx, - smb_fname, - &parent_name, - &file_name); - if (!NT_STATUS_IS_OK(status)) { - goto err; - } - - if (SMB_VFS_STAT(conn, parent_name) != 0) { - status = map_nt_error_from_unix(errno); - goto err; - } - /* Remember where we were. */ - saved_dir_fname = vfs_GetWd(ctx, conn); - if (!saved_dir_fname) { - status = map_nt_error_from_unix(errno); - goto err; - } - - if (vfs_ChDir(conn, parent_name) == -1) { - status = map_nt_error_from_unix(errno); - goto err; - } - - smb_fname_cwd = synthetic_smb_fname(talloc_tos(), - ".", - NULL, - NULL, - parent_name->twrp, - 0); - if (smb_fname_cwd == NULL) { - status = NT_STATUS_NO_MEMORY; - goto err; - } - - /* Get the absolute path of the parent directory. */ - resolved_fname = SMB_VFS_REALPATH(conn, ctx, smb_fname_cwd); - if (resolved_fname == NULL) { - status = map_nt_error_from_unix(errno); - goto err; - } - resolved_name = resolved_fname->base_name; - - if (*resolved_name != '/') { - DEBUG(0,("check_reduced_name_with_privilege: realpath " - "doesn't return absolute paths !\n")); - status = NT_STATUS_OBJECT_NAME_INVALID; - goto err; - } - - DBG_DEBUG("realpath [%s] -> [%s]\n", - smb_fname_str_dbg(parent_name), - resolved_name); - - /* Now check the stat value is the same. */ - if (SMB_VFS_LSTAT(conn, smb_fname_cwd) != 0) { - status = map_nt_error_from_unix(errno); - goto err; - } - - /* Ensure we're pointing at the same place. */ - if (!check_same_stat(&smb_fname_cwd->st, &parent_name->st)) { - DBG_ERR("device/inode/uid/gid on directory %s changed. " - "Denying access !\n", - smb_fname_str_dbg(parent_name)); - status = NT_STATUS_ACCESS_DENIED; - goto err; - } - - /* Ensure we're below the connect path. */ - - conn_rootdir = SMB_VFS_CONNECTPATH(conn, smb_fname); - if (conn_rootdir == NULL) { - DEBUG(2, ("check_reduced_name_with_privilege: Could not get " - "conn_rootdir\n")); - status = NT_STATUS_ACCESS_DENIED; - goto err; - } - - rootdir_len = strlen(conn_rootdir); - - /* - * In the case of rootdir_len == 1, we know that conn_rootdir is - * "/", and we also know that resolved_name starts with a slash. - * So, in this corner case, resolved_name is automatically a - * sub-directory of the conn_rootdir. Thus we can skip the string - * comparison and the next character checks (which are even - * wrong in this case). - */ - if (rootdir_len != 1) { - bool matched; - - matched = (strncmp(conn_rootdir, resolved_name, - rootdir_len) == 0); - - if (!matched || (resolved_name[rootdir_len] != '/' && - resolved_name[rootdir_len] != '\0')) { - DBG_WARNING("%s is a symlink outside the " - "share path\n", - smb_fname_str_dbg(parent_name)); - DEBUGADD(1, ("conn_rootdir =%s\n", conn_rootdir)); - DEBUGADD(1, ("resolved_name=%s\n", resolved_name)); - status = NT_STATUS_ACCESS_DENIED; - goto err; - } - } - - /* Now ensure that the last component either doesn't - exist, or is *NOT* a symlink. */ - - ret = SMB_VFS_LSTAT(conn, file_name); - if (ret == -1) { - /* Errno must be ENOENT for this be ok. */ - if (errno != ENOENT) { - status = map_nt_error_from_unix(errno); - DBG_WARNING("LSTAT on %s failed with %s\n", - smb_fname_str_dbg(file_name), - nt_errstr(status)); - goto err; - } - } - - if (VALID_STAT(file_name->st) && - S_ISLNK(file_name->st.st_ex_mode)) - { - DBG_WARNING("Last component %s is a symlink. Denying" - "access.\n", - smb_fname_str_dbg(file_name)); - status = NT_STATUS_ACCESS_DENIED; - goto err; - } - - status = NT_STATUS_OK; - - err: - - if (saved_dir_fname != NULL) { - vfs_ChDir(conn, saved_dir_fname); - TALLOC_FREE(saved_dir_fname); - } - TALLOC_FREE(resolved_fname); - TALLOC_FREE(parent_name); - return status; -} - /******************************************************************* Reduce a file name, removing .. elements and checking that it is below dir in the hierarchy. This uses realpath. -- 2.25.1 From f8ecb37606ef65a53fbf45c7a4305454de1e53af Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 10:19:38 -0800 Subject: [PATCH 60/99] CVE-2021-44141: s3: smbd: filename_convert() is now a one-to-one wrapper around filename_convert_internal(). Remove filename_convert() and rename filename_convert_internal() -> filename_convert(). Move the old DEBUG(..) statements to DBG_XXX() so they don't print the wrong name. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 46 +++++++++++------------------------------ 1 file changed, 12 insertions(+), 34 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 297f48496e8..2c4fa5506ff 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1913,13 +1913,12 @@ char *get_original_lcomp(TALLOC_CTX *ctx, * @return NT_STATUS_OK if all operations completed successfully, appropriate * error otherwise. */ -static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, - connection_struct *conn, - struct smb_request *smbreq, - const char *name_in, - uint32_t ucf_flags, - NTTIME twrp, - struct smb_filename **_smb_fname) +NTSTATUS filename_convert(TALLOC_CTX *ctx, + connection_struct *conn, + const char *name_in, + uint32_t ucf_flags, + NTTIME twrp, + struct smb_filename **_smb_fname) { struct smb_filename *smb_fname = NULL; bool has_wild; @@ -1935,10 +1934,10 @@ static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, !conn->sconn->using_smb2, &fname); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10,("filename_convert_internal: dfs_redirect " + DBG_DEBUG("dfs_redirect " "failed for name %s with %s\n", name_in, - nt_errstr(status) )); + nt_errstr(status)); return status; } name_in = fname; @@ -1964,10 +1963,10 @@ static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, status = unix_convert(ctx, conn, name_in, twrp, &smb_fname, ucf_flags); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10,("filename_convert_internal: unix_convert failed " + DBG_DEBUG("unix_convert failed " "for name %s with %s\n", name_in, - nt_errstr(status) )); + nt_errstr(status)); return status; } @@ -1984,10 +1983,10 @@ static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, status = check_name(conn, smb_fname); } if (!NT_STATUS_IS_OK(status)) { - DEBUG(3,("filename_convert_internal: check_name failed " + DBG_NOTICE("check_name failed " "for name %s with %s\n", smb_fname_str_dbg(smb_fname), - nt_errstr(status) )); + nt_errstr(status)); TALLOC_FREE(smb_fname); return status; } @@ -2035,27 +2034,6 @@ static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, return status; } -/* - * Go through all the steps to validate a filename. - * Non-root version. - */ - -NTSTATUS filename_convert(TALLOC_CTX *ctx, - connection_struct *conn, - const char *name_in, - uint32_t ucf_flags, - NTTIME twrp, - struct smb_filename **pp_smb_fname) -{ - return filename_convert_internal(ctx, - conn, - NULL, - name_in, - ucf_flags, - twrp, - pp_smb_fname); -} - /* * Build the full path from a dirfsp and dirfsp relative name */ -- 2.25.1 From 3490db2a38981b10ad165d9815ff026ad1b8513d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 16:00:26 -0800 Subject: [PATCH 61/99] CVE-2021-44141: s3: smbd: In dfs_path_lookup(). If we have a DFS path including a @GMT-token, don't throw away the twrp value when parsing the path. Not yet used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/msdfs.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index fd002e98071..86ca79a994b 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -697,6 +697,7 @@ static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, const struct dfs_path *pdp, /* Parsed out server+share+extrapath. */ uint32_t ucf_flags, + NTTIME *_twrp, int *consumedcntp, struct referral **ppreflist, size_t *preferral_count) @@ -867,6 +868,10 @@ static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, } } + if ((ucf_flags & UCF_GMT_PATHNAME) && _twrp != NULL) { + *_twrp = smb_fname->twrp; + } + status = NT_STATUS_OK; out: @@ -965,6 +970,7 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, path_in, pdp, ucf_flags, + NULL, /* twrp. */ NULL, /* int *consumedcntp */ NULL, /* struct referral **ppreflist */ NULL); /* size_t *preferral_count */ @@ -1189,6 +1195,7 @@ NTSTATUS get_referred_path(TALLOC_CTX *ctx, dfs_path, pdp, 0, /* ucf_flags */ + NULL, consumedcntp, &jucn->referral_list, &jucn->referral_count); -- 2.25.1 From 5c55cd93e5bd1481e88edd4fa0c76f4679bdfcc6 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 16:14:08 -0800 Subject: [PATCH 62/99] CVE-2021-44141: s3: smbd: Allow dfs_redirect() to return a TWRP token it got from a parsed pathname. This one is subtle. If an SMB1 request has both a DFS path and a @GMT token, the unix_convert() inside the DFS path processing will remove the @GMT token, not allowing the subsequent unix_convert() inside filename_convert() to see it. By returning it from dfs_redirect() we can ensure it's correctly added to the smb_filename returned from filename_convert(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 11 +++++++++-- source3/smbd/msdfs.c | 3 ++- source3/smbd/proto.h | 1 + 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 2c4fa5506ff..bcbe9f70015 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1824,6 +1824,7 @@ char *get_original_lcomp(TALLOC_CTX *ctx, char *last_slash = NULL; char *orig_lcomp; char *fname = NULL; + NTTIME twrp = 0; NTSTATUS status; if (ucf_flags & UCF_DFS_PATHNAME) { @@ -1832,6 +1833,7 @@ char *get_original_lcomp(TALLOC_CTX *ctx, filename_in, ucf_flags, !conn->sconn->using_smb2, + &twrp, &fname); if (!NT_STATUS_IS_OK(status)) { DBG_DEBUG("dfs_redirect " @@ -1860,7 +1862,7 @@ char *get_original_lcomp(TALLOC_CTX *ctx, filename_in, NULL, NULL, - 0, + twrp, 0); if (smb_fname == NULL) { TALLOC_FREE(fname); @@ -1868,7 +1870,7 @@ char *get_original_lcomp(TALLOC_CTX *ctx, } status = canonicalize_snapshot_path(smb_fname, ucf_flags, - 0); + twrp); if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(fname); TALLOC_FREE(smb_fname); @@ -1928,10 +1930,12 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, if (ucf_flags & UCF_DFS_PATHNAME) { char *fname = NULL; + NTTIME dfs_twrp = 0; status = dfs_redirect(ctx, conn, name_in, ucf_flags, !conn->sconn->using_smb2, + &dfs_twrp, &fname); if (!NT_STATUS_IS_OK(status)) { DBG_DEBUG("dfs_redirect " @@ -1942,6 +1946,9 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, } name_in = fname; ucf_flags &= ~UCF_DFS_PATHNAME; + if (twrp == 0 && dfs_twrp != 0) { + twrp = dfs_twrp; + } } if (is_fake_file_path(name_in)) { diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 86ca79a994b..07636592016 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -901,6 +901,7 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, const char *path_in, uint32_t ucf_flags, bool allow_broken_path, + NTTIME *_twrp, char **pp_path_out) { const struct loadparm_substitution *lp_sub = @@ -970,7 +971,7 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, path_in, pdp, ucf_flags, - NULL, /* twrp. */ + _twrp, /* twrp. */ NULL, /* int *consumedcntp */ NULL, /* struct referral **ppreflist */ NULL); /* size_t *preferral_count */ diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 5e7bc392b87..c1db119633e 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -567,6 +567,7 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, const char *name_in, uint32_t ucf_flags, bool allow_broken_path, + NTTIME *twrp, char **pp_name_out); struct connection_struct; struct smb_filename; -- 2.25.1 From e6d9ef3b1e8e19c1b02a3320a619464b1c319a51 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 10:35:09 -0800 Subject: [PATCH 63/99] CVE-2021-44141: s3: smbd: Add filename_convert_smb1_search_path() - deals with SMB1 search pathnames. SMB1search and trans2 findfirst are unique in that they are the only passed in pathnames that can contain a terminal wildcard component. Deal with these two special cases with this new function that strips off the terminal wildcard and returns as the mask, and pass the non-wildcard parent directory component through the standard filename_convert(). Uses new helper function strip_gmt_from_raw_dfs(). When SMB1search and trans2 findfirst have been converted to use this function, we can strip all wildcard handling out of filename_convert() as we now know it will only ever be given valid pathnames. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 240 ++++++++++++++++++++++++++++++++++++++++ source3/smbd/proto.h | 6 + 2 files changed, 246 insertions(+) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index bcbe9f70015..ab2a418a3a0 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -2041,6 +2041,246 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, return status; } +/* + * Strip a @GMT component from an SMB1-DFS path. Could be anywhere + * in the path. + */ + +static char *strip_gmt_from_raw_dfs(TALLOC_CTX *ctx, + const char *name_in, + bool posix_pathnames, + NTTIME *_twrp) +{ + NTSTATUS status; + struct smb_filename *smb_fname = NULL; + char *name_out = NULL; + + smb_fname = synthetic_smb_fname(ctx, + name_in, + NULL, + NULL, + 0, + 0); + if (smb_fname == NULL) { + return NULL; + } + if (!posix_pathnames) { + /* + * Raw DFS names are still '\\' separated. + * canonicalize_snapshot_path() only works + * on '/' separated paths. Convert. + */ + string_replace(smb_fname->base_name, '\\', '/'); + } + status = canonicalize_snapshot_path(smb_fname, + UCF_GMT_PATHNAME, + 0); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(smb_fname); + return NULL; + } + if (!posix_pathnames) { + /* Replace as raw DFS names. */ + string_replace(smb_fname->base_name, '/', '\\'); + } + name_out = talloc_strdup(ctx, smb_fname->base_name); + *_twrp = smb_fname->twrp; + TALLOC_FREE(smb_fname); + return name_out; +} + +/* + * Deal with the SMB1 semantics of sending a pathname with a + * wildcard as the terminal component for a SMB1search or + * trans2 findfirst. + */ + +NTSTATUS filename_convert_smb1_search_path(TALLOC_CTX *ctx, + connection_struct *conn, + const char *name_in, + uint32_t ucf_flags, + struct smb_filename **_smb_fname_out, + char **_mask_out) +{ + NTSTATUS status; + char *p = NULL; + char *mask = NULL; + struct smb_filename *smb_fname = NULL; + bool posix_pathnames = (ucf_flags & UCF_POSIX_PATHNAMES); + NTTIME twrp = 0; + TALLOC_CTX *frame = talloc_stackframe(); + + *_smb_fname_out = NULL; + *_mask_out = NULL; + + DBG_DEBUG("name_in: %s\n", name_in); + + if (ucf_flags & UCF_DFS_PATHNAME) { + /* + * We've been given a raw DFS pathname. + * In Windows mode this is separated by '\\' + * characters. + * + * We need to remove the last component + * which must be a wildcard before passing + * to dfs_redirect(). But the last component + * may also be a @GMT- token so we have to + * remove that first. + */ + char path_sep = posix_pathnames ? '/' : '\\'; + char *fname = NULL; + char *name_in_copy = NULL; + char *last_component = NULL; + + /* Work on a copy of name_in. */ + if (ucf_flags & UCF_GMT_PATHNAME) { + name_in_copy = strip_gmt_from_raw_dfs(frame, + name_in, + posix_pathnames, + &twrp); + ucf_flags &= ~UCF_GMT_PATHNAME; + } else { + name_in_copy = talloc_strdup(frame, name_in); + } + if (name_in_copy == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + + /* + * Now we know that the last component is the + * wildcard. Copy it and truncate to remove it. + */ + p = strrchr_m(name_in_copy, path_sep); + if (p == NULL) { + last_component = talloc_strdup(frame, name_in_copy); + name_in_copy[0] = '\0'; + } else { + last_component = talloc_strdup(frame, p+1); + *p = '\0'; + } + if (last_component == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + + DBG_DEBUG("name_in_copy: %s\n", name_in); + + /* + * Now we can call dfs_redirect() + * on the name without wildcard. + */ + status = dfs_redirect(frame, + conn, + name_in_copy, + ucf_flags, + !conn->sconn->using_smb2, + NULL, + &fname); + if (!NT_STATUS_IS_OK(status)) { + DBG_DEBUG("dfs_redirect " + "failed for name %s with %s\n", + name_in_copy, + nt_errstr(status)); + TALLOC_FREE(frame); + return status; + } + /* Add the last component back. */ + if (fname[0] == '\0') { + name_in = talloc_strdup(frame, last_component); + } else { + name_in = talloc_asprintf(frame, + "%s%c%s", + fname, + path_sep, + last_component); + } + if (name_in == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + ucf_flags &= ~UCF_DFS_PATHNAME; + + DBG_DEBUG("After DFS redirect name_in: %s\n", name_in); + } + + smb_fname = synthetic_smb_fname(frame, + name_in, + NULL, + NULL, + twrp, + posix_pathnames ? + SMB_FILENAME_POSIX_PATH : 0); + if (smb_fname == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + + /* Canonicalize any @GMT- paths. */ + status = canonicalize_snapshot_path(smb_fname, ucf_flags, twrp); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(frame); + return status; + } + + /* Get the original lcomp. */ + mask = get_original_lcomp(frame, + conn, + name_in, + ucf_flags); + if (mask == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + + if (mask[0] == '\0') { + /* Windows and OS/2 systems treat search on the root as * */ + TALLOC_FREE(mask); + mask = talloc_strdup(frame, "*"); + if (mask == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + } + + DBG_DEBUG("mask = %s\n", mask); + + /* + * Remove the terminal component so + * filename_convert never sees the mask. + */ + p = strrchr_m(smb_fname->base_name,'/'); + if (p == NULL) { + /* filename_convert handles a '\0' base_name. */ + smb_fname->base_name[0] = '\0'; + } else { + *p = '\0'; + } + + DBG_DEBUG("For filename_convert: smb_fname = %s\n", + smb_fname_str_dbg(smb_fname)); + + /* Convert the parent directory path. */ + status = filename_convert(frame, + conn, + smb_fname->base_name, + ucf_flags, + smb_fname->twrp, + &smb_fname); + + if (NT_STATUS_IS_OK(status)) { + *_smb_fname_out = talloc_move(ctx, &smb_fname); + *_mask_out = talloc_move(ctx, &mask); + } else { + DBG_DEBUG("filename_convert error for %s: %s\n", + smb_fname_str_dbg(smb_fname), + nt_errstr(status)); + } + + TALLOC_FREE(frame); + return status; +} + /* * Build the full path from a dirfsp and dirfsp relative name */ diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index c1db119633e..46d3f2e5156 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -382,6 +382,12 @@ NTSTATUS filename_convert(TALLOC_CTX *mem_ctx, uint32_t ucf_flags, NTTIME twrp, struct smb_filename **pp_smb_fname); +NTSTATUS filename_convert_smb1_search_path(TALLOC_CTX *ctx, + connection_struct *conn, + const char *name_in, + uint32_t ucf_flags, + struct smb_filename **_smb_fname_out, + char **_mask_out); /* The following definitions come from smbd/files.c */ -- 2.25.1 From 0f1436ed031b702ab5853b6a21e476a1c47b243c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:22:03 -0800 Subject: [PATCH 64/99] CVE-2021-44141: s3: smbd: Convert reply_search() to use filename_convert_smb1_search_path(). Cleans up this code path nicely ! BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 71 ++++++++++---------------------------------- 1 file changed, 16 insertions(+), 55 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index d2048856cff..cc95bfa3af0 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -1747,15 +1747,17 @@ void reply_search(struct smb_request *req) /* dirtype &= ~FILE_ATTRIBUTE_DIRECTORY; */ if (status_len == 0) { - int ret; + const char *dirpath; struct smb_filename *smb_dname = NULL; - uint32_t ucf_flags = UCF_ALWAYS_ALLOW_WCARD_LCOMP | - ucf_flags_from_smb_request(req); - nt_status = filename_convert(ctx, conn, - path, - ucf_flags, - 0, - &smb_fname); + uint32_t ucf_flags = ucf_flags_from_smb_request(req); + + nt_status = filename_convert_smb1_search_path(ctx, + conn, + path, + ucf_flags, + &smb_dname, + &mask); + if (!NT_STATUS_IS_OK(nt_status)) { if (NT_STATUS_EQUAL(nt_status,NT_STATUS_PATH_NOT_COVERED)) { reply_botherror(req, NT_STATUS_PATH_NOT_COVERED, @@ -1766,56 +1768,9 @@ void reply_search(struct smb_request *req) goto out; } - directory = smb_fname->base_name; - - p = strrchr_m(directory,'/'); - if ((p != NULL) && (*directory != '/')) { - mask = talloc_strdup(ctx, p + 1); - directory = talloc_strndup(ctx, directory, - PTR_DIFF(p, directory)); - } else { - mask = talloc_strdup(ctx, directory); - directory = talloc_strdup(ctx,"."); - } - - if (!directory) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - memset((char *)status,'\0',21); SCVAL(status,0,(dirtype & 0x1F)); - smb_dname = synthetic_smb_fname(talloc_tos(), - directory, - NULL, - NULL, - smb_fname->twrp, - smb_fname->flags); - if (smb_dname == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - - /* - * As we've cut off the last component from - * smb_fname we need to re-stat smb_dname - * so FILE_OPEN disposition knows the directory - * exists. - */ - ret = vfs_stat(conn, smb_dname); - if (ret == -1) { - nt_status = map_nt_error_from_unix(errno); - reply_nterror(req, nt_status); - goto out; - } - - nt_status = openat_pathref_fsp(conn->cwd_fsp, smb_dname); - if (!NT_STATUS_IS_OK(nt_status)) { - reply_nterror(req, nt_status); - goto out; - } - /* * Open an fsp on this directory for the dptr. */ @@ -1872,6 +1827,12 @@ void reply_search(struct smb_request *req) } dptr_num = dptr_dnum(fsp->dptr); + dirpath = dptr_path(sconn, dptr_num); + directory = talloc_strdup(ctx, dirpath); + if (!directory) { + reply_nterror(req, NT_STATUS_NO_MEMORY); + goto out; + } } else { int status_dirtype; -- 2.25.1 From 12b44645fb92de451cf82de12b46a43fdc1c2cc1 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:28:40 -0800 Subject: [PATCH 65/99] CVE-2021-44141: s3: smbd: Fix call_trans2findfirst() to use filename_convert_smb1_search_path(). filename_convert() no longer has to handle wildcards. UCF_ALWAYS_ALLOW_WCARD_LCOMP is now unused. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/trans2.c | 99 ++++--------------------------------------- 1 file changed, 8 insertions(+), 91 deletions(-) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 4612221dbfe..3ba7011b989 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -2665,14 +2665,12 @@ static void call_trans2findfirst(connection_struct *conn, NTSTATUS ntstatus = NT_STATUS_OK; bool ask_sharemode = lp_smbd_search_ask_sharemode(SNUM(conn)); struct smbd_server_connection *sconn = req->sconn; - uint32_t ucf_flags = UCF_ALWAYS_ALLOW_WCARD_LCOMP | - ucf_flags_from_smb_request(req); + uint32_t ucf_flags = ucf_flags_from_smb_request(req); bool backup_priv = false; bool as_root = false; files_struct *fsp = NULL; const struct loadparm_substitution *lp_sub = loadparm_s3_global_substitution(); - int ret; if (total_params < 13) { reply_nterror(req, NT_STATUS_INVALID_PARAMETER); @@ -2758,11 +2756,12 @@ close_if_end = %d requires_resume_key = %d backup_priv = %d level = 0x%x, max_da become_root(); as_root = true; } - ntstatus = filename_convert(talloc_tos(), conn, - directory, - ucf_flags, - 0, - &smb_dname); + ntstatus = filename_convert_smb1_search_path(talloc_tos(), + conn, + directory, + ucf_flags, + &smb_dname, + &mask); if (!NT_STATUS_IS_OK(ntstatus)) { if (NT_STATUS_EQUAL(ntstatus,NT_STATUS_PATH_NOT_COVERED)) { @@ -2774,72 +2773,9 @@ close_if_end = %d requires_resume_key = %d backup_priv = %d level = 0x%x, max_da goto out; } - /* - * The above call to filename_convert() is on the path from the client - * including the search mask. Until the code that chops of the search - * mask from the path below is moved before the call to - * filename_convert(), we close a possible pathref fsp to ensure - * SMB_VFS_CREATE_FILE() below will internally open a pathref fsp on the - * correct path. - */ - if (smb_dname->fsp != NULL) { - ntstatus = fd_close(smb_dname->fsp); - if (!NT_STATUS_IS_OK(ntstatus)) { - reply_nterror(req, ntstatus); - goto out; - } - /* - * The pathref fsp link destructor will set smb_dname->fsp to - * NULL. Turning this into an assert to give a hint at readers - * of the code trying to understand the mechanics. - */ - file_free(req, smb_dname->fsp); - SMB_ASSERT(smb_dname->fsp == NULL); - } - - mask = get_original_lcomp(talloc_tos(), - conn, - directory, - ucf_flags); - if (mask == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - + TALLOC_FREE(directory); directory = smb_dname->base_name; - p = strrchr_m(directory,'/'); - if(p == NULL) { - /* Windows and OS/2 systems treat search on the root '\' as if it were '\*' */ - if((directory[0] == '.') && (directory[1] == '\0')) { - mask = talloc_strdup(talloc_tos(),"*"); - if (!mask) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - } - } else { - *p = 0; - } - - if (p == NULL || p == directory) { - struct smb_filename *old_name = smb_dname; - - /* Ensure we don't have a directory name of "". */ - smb_dname = synthetic_smb_fname(talloc_tos(), - ".", - NULL, - &old_name->st, - old_name->twrp, - old_name->flags); - TALLOC_FREE(old_name); - if (smb_dname == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - directory = smb_dname->base_name; - } - DEBUG(5,("dir=%s, mask = %s\n",directory, mask)); if (info_level == SMB_FIND_EA_LIST) { @@ -2897,25 +2833,6 @@ total_data=%u (should be %u)\n", (unsigned int)total_data, (unsigned int)IVAL(pd } params = *pparams; - /* - * As we've cut off the last component from - * smb_fname we need to re-stat smb_dname - * so FILE_OPEN disposition knows the directory - * exists. - */ - ret = vfs_stat(conn, smb_dname); - if (ret == -1) { - ntstatus = map_nt_error_from_unix(errno); - reply_nterror(req, ntstatus); - goto out; - } - - ntstatus = openat_pathref_fsp(conn->cwd_fsp, smb_dname); - if (!NT_STATUS_IS_OK(ntstatus)) { - reply_nterror(req, ntstatus); - goto out; - } - /* * Open an fsp on this directory for the dptr. */ -- 2.25.1 From fc8e6669edb9e20fbc3a4f06dccccbb7ec676f70 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:31:40 -0800 Subject: [PATCH 66/99] CVE-2021-44141: s3: smbd: dfs_path_lookup() no longer deals with wildcards. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/msdfs.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 07636592016..6a3abd2f6eb 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -680,10 +680,6 @@ bool is_msdfs_link(struct files_struct *dirfsp, Used by other functions to decide if a dfs path is remote, and to get the list of referred locations for that remote path. - search_flag: For findfirsts, dfs links themselves are not - redirected, but paths beyond the links are. For normal smb calls, - even dfs links need to be redirected. - consumedcntp: how much of the dfs path is being redirected. the client should try the remaining path on the redirected server. @@ -755,15 +751,6 @@ static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, TALLOC_FREE(parent_fname); if (NT_STATUS_IS_OK(status)) { - /* XX_ALLOW_WCARD_XXX is called from search functions.*/ - if (ucf_flags & UCF_ALWAYS_ALLOW_WCARD_LCOMP) { - DBG_INFO("(FindFirst) No redirection " - "for dfs link %s.\n", - dfspath); - status = NT_STATUS_OK; - goto out; - } - DBG_INFO("%s resolves to a valid dfs link\n", dfspath); -- 2.25.1 From d91d4a17443ab833bd210c10ac68b3992cb97370 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:42:23 -0800 Subject: [PATCH 67/99] CVE-2021-44141: s3: smbd: Remove 'bool search_wcard_flag' from parse_dfs_path(). Never set. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/msdfs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 6a3abd2f6eb..1feeec6fff2 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -894,14 +894,13 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, const struct loadparm_substitution *lp_sub = loadparm_s3_global_substitution(); NTSTATUS status; - bool search_wcard_flag = (ucf_flags & UCF_ALWAYS_ALLOW_WCARD_LCOMP); struct dfs_path *pdp = talloc(ctx, struct dfs_path); if (!pdp) { return NT_STATUS_NO_MEMORY; } - status = parse_dfs_path(conn, path_in, search_wcard_flag, + status = parse_dfs_path(conn, path_in, false, allow_broken_path, pdp); if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(pdp); -- 2.25.1 From 6f2c67d9993925e45245c7c3f1aa947d72cd2573 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:48:23 -0800 Subject: [PATCH 68/99] CVE-2021-44141: s3: smbd: parse_dfs_path() can ignore wildcards. If one is passed to filename_convert(), it will error out there with NT_STATUS_OBJECT_NAME_INVALID. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/msdfs.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 1feeec6fff2..c003b442baa 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -215,12 +215,6 @@ static NTSTATUS parse_dfs_path(connection_struct *conn, if (pdp->posix_path) { status = check_path_syntax_posix(pdp->reqpath); } else { - if (!allow_wcards) { - bool has_wcard = ms_has_wild(pdp->reqpath); - if (has_wcard) { - return NT_STATUS_INVALID_PARAMETER; - } - } status = check_path_syntax(pdp->reqpath); } -- 2.25.1 From b73be0c7a7c86943416cb83de387341ebfb169fd Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:30:42 -0800 Subject: [PATCH 69/99] CVE-2021-44141: s3: smbd: filename_convert() no longer deals with wildcards. These are already errored out with NT_STATUS_OBJECT_NAME_INVALID in the unix_convert() code. Remove the check. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index ab2a418a3a0..dca5de31be6 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1923,7 +1923,6 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, struct smb_filename **_smb_fname) { struct smb_filename *smb_fname = NULL; - bool has_wild; NTSTATUS status; *_smb_fname = NULL; @@ -1998,14 +1997,6 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, return status; } - has_wild = ms_has_wild(name_in); - if (has_wild) { - DBG_DEBUG("[%s] contains wildcard, skipping pathref fsp\n", - name_in); - *_smb_fname = smb_fname; - return NT_STATUS_OK; - } - if (!VALID_STAT(smb_fname->st)) { DBG_DEBUG("[%s] does not exist, skipping pathref fsp\n", smb_fname_str_dbg(smb_fname)); -- 2.25.1 From 5e42ab3f6a09ec469ef882dca24f1372711646a0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:33:42 -0800 Subject: [PATCH 70/99] CVE-2021-44141: s3: smbd: Inside 'struct uc_state', remove allow_wcard_last_component. This is never allowed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index dca5de31be6..dc2a5a33824 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -524,7 +524,6 @@ struct uc_state { bool component_was_mangled; bool name_has_wildcard; bool posix_pathnames; - bool allow_wcard_last_component; bool done; }; @@ -870,7 +869,7 @@ static NTSTATUS unix_convert_step(struct uc_state *state) return NT_STATUS_OBJECT_NAME_INVALID; } return determine_path_error(state->end+1, - state->allow_wcard_last_component, + false, state->posix_pathnames); } @@ -962,7 +961,6 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, .orig_path = orig_path, .ucf_flags = ucf_flags, .posix_pathnames = (ucf_flags & UCF_POSIX_PATHNAMES), - .allow_wcard_last_component = (ucf_flags & UCF_ALWAYS_ALLOW_WCARD_LCOMP), }; *smb_fname_out = NULL; @@ -1038,7 +1036,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, status = NT_STATUS_OBJECT_NAME_INVALID; } else { status =determine_path_error(&state->orig_path[2], - state->allow_wcard_last_component, + false, state->posix_pathnames); } goto err; @@ -1163,7 +1161,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, if (!state->posix_pathnames) { /* POSIX pathnames have no wildcards. */ state->name_has_wildcard = ms_has_wild(state->smb_fname->base_name); - if (state->name_has_wildcard && !state->allow_wcard_last_component) { + if (state->name_has_wildcard) { /* Wildcard not valid anywhere. */ status = NT_STATUS_OBJECT_NAME_INVALID; goto fail; -- 2.25.1 From b0fc0efbac5b1c4144769ec5a2855f4276b9c7a2 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 12:37:15 -0800 Subject: [PATCH 71/99] CVE-2021-44141: s3: smbd: We no longer need determine_path_error(). Now we don't have to consider wildcards just return NT_STATUS_OBJECT_PATH_NOT_FOUND for the cases we used to call it. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 47 ++--------------------------------------- 1 file changed, 2 insertions(+), 45 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index dc2a5a33824..09bf859bd2d 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -90,45 +90,6 @@ static bool mangled_equal(const char *name1, return strequal(name1, mname); } -/**************************************************************************** - Cope with the differing wildcard and non-wildcard error cases. -****************************************************************************/ - -static NTSTATUS determine_path_error(const char *name, - bool allow_wcard_last_component, - bool posix_pathnames) -{ - const char *p; - bool name_has_wild = false; - - if (!allow_wcard_last_component) { - /* Error code within a pathname. */ - return NT_STATUS_OBJECT_PATH_NOT_FOUND; - } - - /* We're terminating here so we - * can be a little slower and get - * the error code right. Windows - * treats the last part of the pathname - * separately I think, so if the last - * component is a wildcard then we treat - * this ./ as "end of component" */ - - p = strchr(name, '/'); - - if (!posix_pathnames) { - name_has_wild = ms_has_wild(name); - } - - if (!p && (name_has_wild || ISDOT(name))) { - /* Error code at the end of a pathname. */ - return NT_STATUS_OBJECT_NAME_INVALID; - } else { - /* Error code within a pathname. */ - return NT_STATUS_OBJECT_PATH_NOT_FOUND; - } -} - static NTSTATUS check_for_dot_component(const struct smb_filename *smb_fname) { /* Ensure we catch all names with in "/." @@ -868,9 +829,7 @@ static NTSTATUS unix_convert_step(struct uc_state *state) /* Error code at the end of a pathname. */ return NT_STATUS_OBJECT_NAME_INVALID; } - return determine_path_error(state->end+1, - false, - state->posix_pathnames); + return NT_STATUS_OBJECT_PATH_NOT_FOUND; } /* The name cannot have a wildcard if it's not @@ -1035,9 +994,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, if (state->orig_path[1] == '\0' || state->orig_path[2] == '\0') { status = NT_STATUS_OBJECT_NAME_INVALID; } else { - status =determine_path_error(&state->orig_path[2], - false, - state->posix_pathnames); + status = NT_STATUS_OBJECT_PATH_NOT_FOUND; } goto err; } -- 2.25.1 From d52dd78e9d8cecbc9e913c0b91f345cafe755dbd Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 12:40:43 -0800 Subject: [PATCH 72/99] CVE-2021-44141: s3: smbd: UCF_ALWAYS_ALLOW_WCARD_LCOMP 0x00000002 is no longer used. Hurrah ! BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 3 --- source3/smbd/smbd.h | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 09bf859bd2d..73a2f116915 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -459,9 +459,6 @@ Note NT_STATUS_OK doesn't mean the name exists or is valid, just that we didn't get any fatal errors that should immediately terminate the calling SMB processing whilst resolving. -If UCF_ALWAYS_ALLOW_WCARD_LCOMP is passed in, then a MS wildcard -should be allowed in the last component of the path only. - If the orig_path was a stream, smb_filename->base_name will point to the base filename, and smb_filename->stream_name will point to the stream name. If orig_path was not a stream, then smb_filename->stream_name will be NULL. diff --git a/source3/smbd/smbd.h b/source3/smbd/smbd.h index 84fcf033a71..88bff2830d9 100644 --- a/source3/smbd/smbd.h +++ b/source3/smbd/smbd.h @@ -61,7 +61,7 @@ struct trans_state { * unix_convert_flags */ /* UCF_SAVE_LCOMP 0x00000001 is no longer used. */ -#define UCF_ALWAYS_ALLOW_WCARD_LCOMP 0x00000002 +/* UCF_ALWAYS_ALLOW_WCARD_LCOMP 0x00000002 is no longer used. */ /* UCF_COND_ALLOW_WCARD_LCOMP 0x00000004 is no longer used. */ #define UCF_POSIX_PATHNAMES 0x00000008 /* #define UCF_UNIX_NAME_LOOKUP 0x00000010 is no longer used. */ -- 2.25.1 From 3471f03816f8133f501288c8e468c36cdad8ae65 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 12:53:36 -0800 Subject: [PATCH 73/99] CVE-2021-44141: s3: smbd: Inside unix_convert(), never set state->name_is_wildcard. We error out immediately if it's set anyway. Preparing to remove 'state->name_is_wildcard' structure element. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 73a2f116915..2095c23aaf4 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1114,8 +1114,8 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, if (!state->posix_pathnames) { /* POSIX pathnames have no wildcards. */ - state->name_has_wildcard = ms_has_wild(state->smb_fname->base_name); - if (state->name_has_wildcard) { + bool name_has_wildcard = ms_has_wild(state->smb_fname->base_name); + if (name_has_wildcard) { /* Wildcard not valid anywhere. */ status = NT_STATUS_OBJECT_NAME_INVALID; goto fail; -- 2.25.1 From 36f480c7c8ea88238a040415f677ad0a57fec60c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 12:55:41 -0800 Subject: [PATCH 74/99] CVE-2021-44141: s3: smbd: In unix_convert(), remove all references to state->name_has_wildcard. It is never set. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 2095c23aaf4..01c0d9d7dbd 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1125,7 +1125,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, DBG_DEBUG("Begin: name [%s] dirpath [%s] name [%s]\n", state->smb_fname->base_name, state->dirpath, state->name); - if (!state->name_has_wildcard) { + { int parent_stat_errno = 0; /* @@ -1239,27 +1239,6 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, goto done; } } - } else { - /* - * We have a wildcard in the pathname. - * - * Optimization for common case where the wildcard - * is in the last component and the client already - * sent the correct case. - * NOTE : check_parent_exists() doesn't preserve errno. - */ - int saved_errno = errno; - status = check_parent_exists(state->mem_ctx, - state->conn, - state->posix_pathnames, - state->smb_fname, - &state->dirpath, - &state->name, - NULL); - errno = saved_errno; - if (!NT_STATUS_IS_OK(status)) { - goto fail; - } } /* @@ -1299,7 +1278,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, * components as this can change the size. */ - if(!state->component_was_mangled && !state->name_has_wildcard) { + if(!state->component_was_mangled) { stat_cache_add(state->orig_path, state->smb_fname->base_name, state->smb_fname->twrp, -- 2.25.1 From 104499b56ded1960c0fa7f2dfd49eea4d0f76172 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 12:59:50 -0800 Subject: [PATCH 75/99] CVE-2021-44141: s3: smbd: In unix_convert() remove the now unneeded block indentation. We removed the 'if (state->name_has_wildcard) {' clause, so the block no longer needs indenting. Best seen with git show -b. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 193 ++++++++++++++++++++-------------------- 1 file changed, 95 insertions(+), 98 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 01c0d9d7dbd..c8b3f1f5955 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -910,6 +910,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, struct uc_state *state = &uc_state; NTSTATUS status; int ret = -1; + int parent_stat_errno = 0; *state = (struct uc_state) { .mem_ctx = mem_ctx, @@ -1125,119 +1126,115 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, DBG_DEBUG("Begin: name [%s] dirpath [%s] name [%s]\n", state->smb_fname->base_name, state->dirpath, state->name); - { - int parent_stat_errno = 0; + /* + * stat the name - if it exists then we can add the stream back (if + * there was one) and be done! + */ - /* - * stat the name - if it exists then we can add the stream back (if - * there was one) and be done! - */ + ret = vfs_stat(state->conn, state->smb_fname); + if (ret == 0) { + status = check_for_dot_component(state->smb_fname); + if (!NT_STATUS_IS_OK(status)) { + goto fail; + } + /* Add the path (not including the stream) to the cache. */ + stat_cache_add(state->orig_path, + state->smb_fname->base_name, + state->smb_fname->twrp, + state->conn->case_sensitive); + DBG_DEBUG("Conversion of base_name finished " + "[%s] -> [%s]\n", + state->orig_path, state->smb_fname->base_name); + goto done; + } - ret = vfs_stat(state->conn, state->smb_fname); - if (ret == 0) { - status = check_for_dot_component(state->smb_fname); - if (!NT_STATUS_IS_OK(status)) { - goto fail; - } - /* Add the path (not including the stream) to the cache. */ - stat_cache_add(state->orig_path, - state->smb_fname->base_name, - state->smb_fname->twrp, - state->conn->case_sensitive); - DBG_DEBUG("Conversion of base_name finished " - "[%s] -> [%s]\n", - state->orig_path, state->smb_fname->base_name); - goto done; + /* Stat failed - ensure we don't use it. */ + SET_STAT_INVALID(state->smb_fname->st); + + /* + * Note: we must continue processing a path if we get EACCES + * from stat. With NFS4 permissions the file might be lacking + * READ_ATTR, but if the parent has LIST permissions we can + * resolve the path in the path traversal loop down below. + */ + + if (errno == ENOENT) { + /* Optimization when creating a new file - only + the last component doesn't exist. + NOTE : check_parent_exists() doesn't preserve errno. + */ + int saved_errno = errno; + status = check_parent_exists(state->mem_ctx, + state->conn, + state->posix_pathnames, + state->smb_fname, + &state->dirpath, + &state->name, + &parent_stat_errno); + errno = saved_errno; + if (!NT_STATUS_IS_OK(status)) { + goto fail; } + } - /* Stat failed - ensure we don't use it. */ - SET_STAT_INVALID(state->smb_fname->st); + /* + * A special case - if we don't have any wildcards or mangling chars and are case + * sensitive or the underlying filesystem is case insensitive then searching + * won't help. + */ - /* - * Note: we must continue processing a path if we get EACCES - * from stat. With NFS4 permissions the file might be lacking - * READ_ATTR, but if the parent has LIST permissions we can - * resolve the path in the path traversal loop down below. - */ + if ((state->conn->case_sensitive || !(state->conn->fs_capabilities & + FILE_CASE_SENSITIVE_SEARCH)) && + !mangle_is_mangled(state->smb_fname->base_name, state->conn->params)) { - if (errno == ENOENT) { - /* Optimization when creating a new file - only - the last component doesn't exist. - NOTE : check_parent_exists() doesn't preserve errno. - */ - int saved_errno = errno; - status = check_parent_exists(state->mem_ctx, - state->conn, - state->posix_pathnames, - state->smb_fname, - &state->dirpath, - &state->name, - &parent_stat_errno); - errno = saved_errno; - if (!NT_STATUS_IS_OK(status)) { - goto fail; - } + status = check_for_dot_component(state->smb_fname); + if (!NT_STATUS_IS_OK(status)) { + goto fail; } /* - * A special case - if we don't have any wildcards or mangling chars and are case - * sensitive or the underlying filesystem is case insensitive then searching - * won't help. + * The stat failed. Could be ok as it could be + * a new file. */ - if ((state->conn->case_sensitive || !(state->conn->fs_capabilities & - FILE_CASE_SENSITIVE_SEARCH)) && - !mangle_is_mangled(state->smb_fname->base_name, state->conn->params)) { - - status = check_for_dot_component(state->smb_fname); - if (!NT_STATUS_IS_OK(status)) { - goto fail; - } - + if (errno == ENOTDIR || errno == ELOOP) { + status = NT_STATUS_OBJECT_PATH_NOT_FOUND; + goto fail; + } else if (errno == ENOENT) { /* - * The stat failed. Could be ok as it could be - * a new file. - */ - - if (errno == ENOTDIR || errno == ELOOP) { + * Was it a missing last component ? + * or a missing intermediate component ? + * + * Optimization. + * + * For this code path we can guarantee that + * we have gone through check_parent_exists() + * and it returned NT_STATUS_OK. + * + * Either there was no parent component (".") + * parent_stat_errno == 0 and we have a missing + * last component here. + * + * OR check_parent_exists() called STAT/LSTAT + * and if it failed parent_stat_errno has been + * set telling us if the parent existed or not. + * + * Either way we can avoid another STAT/LSTAT + * system call on the parent here. + */ + if (parent_stat_errno == ENOTDIR || + parent_stat_errno == ENOENT || + parent_stat_errno == ELOOP) { status = NT_STATUS_OBJECT_PATH_NOT_FOUND; goto fail; - } else if (errno == ENOENT) { - /* - * Was it a missing last component ? - * or a missing intermediate component ? - * - * Optimization. - * - * For this code path we can guarantee that - * we have gone through check_parent_exists() - * and it returned NT_STATUS_OK. - * - * Either there was no parent component (".") - * parent_stat_errno == 0 and we have a missing - * last component here. - * - * OR check_parent_exists() called STAT/LSTAT - * and if it failed parent_stat_errno has been - * set telling us if the parent existed or not. - * - * Either way we can avoid another STAT/LSTAT - * system call on the parent here. - */ - if (parent_stat_errno == ENOTDIR || - parent_stat_errno == ENOENT || - parent_stat_errno == ELOOP) { - status = NT_STATUS_OBJECT_PATH_NOT_FOUND; - goto fail; - } - - /* - * Missing last component is ok - new file. - * Also deal with permission denied elsewhere. - * Just drop out to done. - */ - goto done; } + + /* + * Missing last component is ok - new file. + * Also deal with permission denied elsewhere. + * Just drop out to done. + */ + goto done; } } -- 2.25.1 From e94d2bcbdc6d4899be71b74a2daf39e65474558c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 13:03:47 -0800 Subject: [PATCH 76/99] CVE-2021-44141: s3: smbd: In unix_convert_step() remove all use of 'state->name_was_wildcard' We know it is never true. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index c8b3f1f5955..c582d24ec6b 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -829,25 +829,6 @@ static NTSTATUS unix_convert_step(struct uc_state *state) return NT_STATUS_OBJECT_PATH_NOT_FOUND; } - /* The name cannot have a wildcard if it's not - the last component. */ - - if (!state->posix_pathnames) { - state->name_has_wildcard = ms_has_wild(state->name); - } - - /* Wildcards never valid within a pathname. */ - if (state->name_has_wildcard && state->end != NULL) { - return NT_STATUS_OBJECT_NAME_INVALID; - } - - /* Skip the stat call if it's a wildcard end. */ - if (state->name_has_wildcard) { - DBG_DEBUG("Wildcard [%s]\n", state->name); - state->done = true; - return NT_STATUS_OK; - } - status = unix_convert_step_stat(state); if (!NT_STATUS_IS_OK(status)) { return status; @@ -880,9 +861,9 @@ static NTSTATUS unix_convert_step(struct uc_state *state) /* * Cache the dirpath thus far. Don't cache a name with mangled - * or wildcard components as this can change the size. + * components as this can change the size. */ - if(!state->component_was_mangled && !state->name_has_wildcard) { + if(!state->component_was_mangled) { stat_cache_add(state->orig_path, state->dirpath, state->smb_fname->twrp, -- 2.25.1 From f77e56e2d1baff6f0ff78e10d6bbba49d106edd9 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 13:05:55 -0800 Subject: [PATCH 77/99] CVE-2021-44141: s3: smbd: In unix_convert_step_stat() remove use of state->name_was_wildcard. It can never be true. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index c582d24ec6b..8f7da13f034 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -662,8 +662,8 @@ static NTSTATUS unix_convert_step_stat(struct uc_state *state) if (state->posix_pathnames) { /* * For posix_pathnames, we're done. - * Don't blunder into the name_has_wildcard OR - * get_real_filename() codepaths as they may + * Don't blunder into the + * get_real_filename() codepath as they may * be doing case insensitive lookups. So when * creating a new POSIX directory Foo they might * match on name foo. @@ -712,10 +712,6 @@ static NTSTATUS unix_convert_step_stat(struct uc_state *state) * Try to find this part of the path in the directory. */ - if (state->name_has_wildcard) { - return unix_convert_step_search_fail(state); - } - dname = (struct smb_filename) { .base_name = state->dirpath, .twrp = state->smb_fname->twrp, -- 2.25.1 From f8698b1f797ddf2c6e418e683e6c68392ad3ef9e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 13:06:27 -0800 Subject: [PATCH 78/99] CVE-2021-44141: s3: smbd: Remove 'struct uc_state' name_has_wildcard element. It is never set or looked at. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 1 - 1 file changed, 1 deletion(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 8f7da13f034..0d82085870c 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -480,7 +480,6 @@ struct uc_state { char *dirpath; char *stream; bool component_was_mangled; - bool name_has_wildcard; bool posix_pathnames; bool done; }; -- 2.25.1 From 10242faa0785ca277d584274c151467e78e787bf Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 12:54:47 -0800 Subject: [PATCH 79/99] CVE-2021-44141: s4: torture: Fix raw.search:test_one_file() to use torture_result() instead of printf. I think this test pre-dates torture_result. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source4/torture/raw/search.c | 60 ++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index 14af01bffad..07285893f40 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -316,7 +316,11 @@ static bool test_one_file(struct torture_context *tctx, fnum = create_complex_file(cli, tctx, fname); if (fnum == -1) { - printf("ERROR: open of %s failed (%s)\n", fname, smbcli_errstr(cli->tree)); + torture_result(tctx, + TORTURE_FAIL, + __location__"ERROR: open of %s failed (%s)\n", + fname, + smbcli_errstr(cli->tree)); ret = false; goto done; } @@ -343,9 +347,11 @@ static bool test_one_file(struct torture_context *tctx, } if (!NT_STATUS_IS_OK(levels[i].status)) { - printf("search level %s(%d) failed - %s\n", - levels[i].name, (int)levels[i].level, - nt_errstr(levels[i].status)); + torture_result(tctx, + TORTURE_FAIL, + __location__"search level %s(%d) failed - %s\n", + levels[i].name, (int)levels[i].level, + nt_errstr(levels[i].status)); ret = false; continue; } @@ -363,7 +369,9 @@ static bool test_one_file(struct torture_context *tctx, expected_status = STATUS_NO_MORE_FILES; } if (!NT_STATUS_EQUAL(status, expected_status)) { - printf("search level %s(%d) should fail with %s - %s\n", + torture_result(tctx, + TORTURE_FAIL, + __location__"search level %s(%d) should fail with %s - %s\n", levels[i].name, (int)levels[i].level, nt_errstr(expected_status), nt_errstr(status)); @@ -400,8 +408,10 @@ static bool test_one_file(struct torture_context *tctx, s = find(name); \ if (s) { \ if ((s->sname1.field1) != (v.sname2.out.field2)) { \ - printf("(%s) %s/%s [0x%x] != %s/%s [0x%x]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [0x%x] != %s/%s [0x%x]\n", \ + __location__, \ #sname1, #field1, (int)s->sname1.field1, \ #sname2, #field2, (int)v.sname2.out.field2); \ ret = false; \ @@ -412,8 +422,10 @@ static bool test_one_file(struct torture_context *tctx, s = find(name); \ if (s) { \ if (s->sname1.field1 != (~1 & nt_time_to_unix(v.sname2.out.field2))) { \ - printf("(%s) %s/%s [%s] != %s/%s [%s]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s/%s [%s]\n", \ + __location__, \ #sname1, #field1, timestring(tctx, s->sname1.field1), \ #sname2, #field2, nt_time_string(tctx, v.sname2.out.field2)); \ ret = false; \ @@ -424,8 +436,10 @@ static bool test_one_file(struct torture_context *tctx, s = find(name); \ if (s) { \ if (s->sname1.field1 != v.sname2.out.field2) { \ - printf("(%s) %s/%s [%s] != %s/%s [%s]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s/%s [%s]\n", \ + __location__, \ #sname1, #field1, nt_time_string(tctx, s->sname1.field1), \ #sname2, #field2, nt_time_string(tctx, v.sname2.out.field2)); \ ret = false; \ @@ -436,8 +450,10 @@ static bool test_one_file(struct torture_context *tctx, s = find(name); \ if (s) { \ if (!s->sname1.field1 || strcmp(s->sname1.field1, v.sname2.out.field2.s)) { \ - printf("(%s) %s/%s [%s] != %s/%s [%s]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s/%s [%s]\n", \ + __location__, \ #sname1, #field1, s->sname1.field1, \ #sname2, #field2, v.sname2.out.field2.s); \ ret = false; \ @@ -450,8 +466,10 @@ static bool test_one_file(struct torture_context *tctx, if (!s->sname1.field1.s || \ strcmp(s->sname1.field1.s, v.sname2.out.field2.s) || \ wire_bad_flags(&s->sname1.field1, flags, cli->transport)) { \ - printf("(%s) %s/%s [%s] != %s/%s [%s]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s/%s [%s]\n", \ + __location__, \ #sname1, #field1, s->sname1.field1.s, \ #sname2, #field2, v.sname2.out.field2.s); \ ret = false; \ @@ -464,8 +482,10 @@ static bool test_one_file(struct torture_context *tctx, if (!s->sname1.field1.s || \ strcmp(s->sname1.field1.s, fname) || \ wire_bad_flags(&s->sname1.field1, flags, cli->transport)) { \ - printf("(%s) %s/%s [%s] != %s\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s\n", \ + __location__, \ #sname1, #field1, s->sname1.field1.s, \ fname); \ ret = false; \ @@ -477,8 +497,10 @@ static bool test_one_file(struct torture_context *tctx, if (s) { \ if (!s->sname1.field1 || \ strcmp(s->sname1.field1, fname)) { \ - printf("(%s) %s/%s [%s] != %s\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s\n", \ + __location__, \ #sname1, #field1, s->sname1.field1, \ fname); \ ret = false; \ -- 2.25.1 From 738c7080e78553b9f6eeef778522a1df9a88f977 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 14:18:47 -0800 Subject: [PATCH 80/99] CVE-2021-44141: s4: torture: In raw.search:test_one_file() remove the leading '\\' in the test filenames. We'll soon be using this under SMB1+POSIX and neither Windows or POSIX need a leading '\\' (and SMB1+POSIX sees the '\\' as part of the name). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source4/torture/raw/search.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index 07285893f40..9b140928689 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -305,8 +305,8 @@ static bool test_one_file(struct torture_context *tctx, { bool ret = true; int fnum; - const char *fname = "\\torture_search.txt"; - const char *fname2 = "\\torture_search-NOTEXIST.txt"; + const char *fname = "torture_search.txt"; + const char *fname2 = "torture_search-NOTEXIST.txt"; NTSTATUS status; int i; union smb_fileinfo all_info, alt_info, name_info, internal_info; @@ -581,20 +581,20 @@ static bool test_one_file(struct torture_context *tctx, short_name, alt_info, alt_name_info, fname, STR_UNICODE); } - CHECK_NAME("STANDARD", standard, name, fname+1, 0); - CHECK_NAME("EA_SIZE", ea_size, name, fname+1, 0); - CHECK_NAME("DIRECTORY_INFO", directory_info, name, fname+1, STR_TERMINATE_ASCII); - CHECK_NAME("FULL_DIRECTORY_INFO", full_directory_info, name, fname+1, STR_TERMINATE_ASCII); + CHECK_NAME("STANDARD", standard, name, fname, 0); + CHECK_NAME("EA_SIZE", ea_size, name, fname, 0); + CHECK_NAME("DIRECTORY_INFO", directory_info, name, fname, STR_TERMINATE_ASCII); + CHECK_NAME("FULL_DIRECTORY_INFO", full_directory_info, name, fname, STR_TERMINATE_ASCII); if (name_info_supported) { - CHECK_NAME("NAME_INFO", name_info, name, fname+1, + CHECK_NAME("NAME_INFO", name_info, name, fname, STR_TERMINATE_ASCII); } - CHECK_NAME("BOTH_DIRECTORY_INFO", both_directory_info, name, fname+1, STR_TERMINATE_ASCII); - CHECK_NAME("ID_FULL_DIRECTORY_INFO", id_full_directory_info, name, fname+1, STR_TERMINATE_ASCII); - CHECK_NAME("ID_BOTH_DIRECTORY_INFO", id_both_directory_info, name, fname+1, STR_TERMINATE_ASCII); - CHECK_UNIX_NAME("UNIX_INFO", unix_info, name, fname+1, STR_TERMINATE_ASCII); + CHECK_NAME("BOTH_DIRECTORY_INFO", both_directory_info, name, fname, STR_TERMINATE_ASCII); + CHECK_NAME("ID_FULL_DIRECTORY_INFO", id_full_directory_info, name, fname, STR_TERMINATE_ASCII); + CHECK_NAME("ID_BOTH_DIRECTORY_INFO", id_both_directory_info, name, fname, STR_TERMINATE_ASCII); + CHECK_UNIX_NAME("UNIX_INFO", unix_info, name, fname, STR_TERMINATE_ASCII); if (internal_info_supported) { CHECK_VAL("ID_FULL_DIRECTORY_INFO", id_full_directory_info, -- 2.25.1 From 4fc4bd4f20cdfcf1df63f76f2f9940808b286c72 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 18 Nov 2021 11:48:42 -0800 Subject: [PATCH 81/99] CVE-2021-44141: s3: smbd: Tighten up info level checks for SMB1+POSIX to make sure POSIX was negotiated first. Add knownfail file knownfail.d/posix_infolevel_fails for tests that don't currently negotiate SMB1+POSIX before using SMB1+POSIX calls. These are: samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* samba3.unix.info2.info2\(nt4_dc_smb1\) samba3.unix.info2.info2\(ad_dc_smb1\) samba3.raw.search.one\ file\ search.* BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 8 +++ source3/smbd/trans2.c | 60 +++++++++++++++++++--- 2 files changed, 62 insertions(+), 6 deletions(-) create mode 100644 selftest/knownfail.d/posix_infolevel_fails diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails new file mode 100644 index 00000000000..78a6781684c --- /dev/null +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -0,0 +1,8 @@ +^samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) +^samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* +^samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* +^samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* +^samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* +^samba3.unix.info2.info2\(nt4_dc_smb1\) +^samba3.unix.info2.info2\(ad_dc_smb1\) +^samba3.raw.search.one\ file\ search.* diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 3ba7011b989..33ba9da9f83 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -2717,6 +2717,10 @@ close_if_end = %d requires_resume_key = %d backup_priv = %d level = 0x%x, max_da reply_nterror(req, NT_STATUS_INVALID_LEVEL); goto out; } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + goto out; + } break; default: reply_nterror(req, NT_STATUS_INVALID_LEVEL); @@ -3183,6 +3187,10 @@ resume_key = %d resume name = %s continue=%d level = %d\n", reply_nterror(req, NT_STATUS_INVALID_LEVEL); return; } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } break; default: reply_nterror(req, NT_STATUS_INVALID_LEVEL); @@ -5144,8 +5152,13 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn, uint32_t access_mask = 0; size_t len = 0; - if (INFO_LEVEL_IS_UNIX(info_level) && !lp_unix_extensions()) { - return NT_STATUS_INVALID_LEVEL; + if (INFO_LEVEL_IS_UNIX(info_level)) { + if (!lp_unix_extensions()) { + return NT_STATUS_INVALID_LEVEL; + } + if (!req->posix_pathnames) { + return NT_STATUS_INVALID_LEVEL; + } } DEBUG(5,("smbd_do_qfilepathinfo: %s (%s) level=%d max_data=%u\n", @@ -5958,9 +5971,15 @@ static void call_trans2qfilepathinfo(connection_struct *conn, DEBUG(3,("call_trans2qfilepathinfo: TRANSACT2_QFILEINFO: level = %d\n", info_level)); - if (INFO_LEVEL_IS_UNIX(info_level) && !lp_unix_extensions()) { - reply_nterror(req, NT_STATUS_INVALID_LEVEL); - return; + if (INFO_LEVEL_IS_UNIX(info_level)) { + if (!lp_unix_extensions()) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } } /* Initial check for valid fsp ptr. */ @@ -6053,6 +6072,10 @@ static void call_trans2qfilepathinfo(connection_struct *conn, reply_nterror(req, NT_STATUS_INVALID_LEVEL); return; } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } } if (req->posix_pathnames) { @@ -9061,7 +9084,9 @@ NTSTATUS smbd_do_setfilepathinfo(connection_struct *conn, if (!lp_unix_extensions()) { return NT_STATUS_INVALID_LEVEL; } - + if (!req->posix_pathnames) { + return NT_STATUS_INVALID_LEVEL; + } status = smbd_do_posix_setfilepathinfo(conn, req, req, @@ -9282,6 +9307,17 @@ static void call_trans2setfilepathinfo(connection_struct *conn, } info_level = SVAL(params,2); + if (INFO_LEVEL_IS_UNIX(info_level)) { + if (!lp_unix_extensions()) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + } + smb_fname = fsp->fsp_name; if (fsp_get_pathref_fd(fsp) == -1) { @@ -9360,6 +9396,18 @@ static void call_trans2setfilepathinfo(connection_struct *conn, } info_level = SVAL(params,0); + + if (INFO_LEVEL_IS_UNIX(info_level)) { + if (!lp_unix_extensions()) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + } + if (req->posix_pathnames) { srvstr_get_path_posix(req, params, -- 2.25.1 From c032a254bb5b703f510c42880ea5416982df9577 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Sat, 20 Nov 2021 20:17:11 -0800 Subject: [PATCH 82/99] CVE-2021-44141: s3: smbclient: Give a message if we try and use any POSIX command without negotiating POSIX first. Ensure we only use a POSIX command if POSIX is set up. Issue the message: Command "posix" must be issued before the "XXXX" command can be used. After the parameter parsing has been done. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/client/client.c | 79 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/source3/client/client.c b/source3/client/client.c index a8e11044b39..5ad6ee7b844 100644 --- a/source3/client/client.c +++ b/source3/client/client.c @@ -2839,6 +2839,11 @@ static int cmd_posix_open(void) d_printf("posix_open 0\n"); return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_open\" command can be used.\n"); + return 1; + } mode = (mode_t)strtol(buf, (char **)NULL, 8); status = cli_resolve_path(ctx, "", @@ -2900,6 +2905,11 @@ static int cmd_posix_mkdir(void) d_printf("posix_mkdir 0\n"); return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_mkdir\" command can be used.\n"); + return 1; + } mode = (mode_t)strtol(buf, (char **)NULL, 8); status = cli_resolve_path(ctx, "", @@ -2934,6 +2944,11 @@ static int cmd_posix_unlink(void) d_printf("posix_unlink \n"); return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_unlink\" command can be used.\n"); + return 1; + } mask = talloc_asprintf(ctx, "%s%s", client_get_cur_dir(), @@ -2979,6 +2994,11 @@ static int cmd_posix_rmdir(void) d_printf("posix_rmdir \n"); return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_rmdir\" command can be used.\n"); + return 1; + } mask = talloc_asprintf(ctx, "%s%s", client_get_cur_dir(), @@ -3178,6 +3198,12 @@ static int cmd_lock(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"lock\" command can be used.\n"); + return 1; + } + len = (uint64_t)strtol(buf, (char **)NULL, 16); status = cli_posix_lock(cli, fnum, start, len, true, lock_type); @@ -3214,6 +3240,12 @@ static int cmd_unlock(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"unlock\" command can be used.\n"); + return 1; + } + len = (uint64_t)strtol(buf, (char **)NULL, 16); status = cli_posix_unlock(cli, fnum, start, len); @@ -3237,6 +3269,12 @@ static int cmd_posix_whoami(void) bool guest = false; uint32_t i; + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_whoami\" command can be used.\n"); + return 1; + } + status = cli_posix_whoami(cli, ctx, &uid, @@ -3374,6 +3412,12 @@ static int cmd_link(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"link\" command can be used.\n"); + return 1; + } + status = cli_posix_hardlink(targetcli, targetname, newname); if (!NT_STATUS_IS_OK(status)) { d_printf("%s linking files (%s -> %s)\n", @@ -3427,6 +3471,12 @@ static int cmd_readlink(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"readlink\" command can be used.\n"); + return 1; + } + status = cli_posix_readlink(targetcli, name, talloc_tos(), &linkname); if (!NT_STATUS_IS_OK(status)) { d_printf("%s readlink on file %s\n", @@ -3466,6 +3516,11 @@ static int cmd_symlink(void) link_target = buf; if (SERVER_HAS_UNIX_CIFS(cli)) { + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"symlink\" command can be used.\n"); + return 1; + } newname = talloc_asprintf(ctx, "%s%s", client_get_cur_dir(), buf2); if (!newname) { @@ -3549,6 +3604,12 @@ static int cmd_chmod(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"chmod\" command can be used.\n"); + return 1; + } + status = cli_posix_chmod(targetcli, targetname, mode); if (!NT_STATUS_IS_OK(status)) { d_printf("%s chmod file %s 0%o\n", @@ -3713,6 +3774,12 @@ static int cmd_getfacl(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"getfacl\" command can be used.\n"); + return 1; + } + status = cli_unix_extensions_version(targetcli, &major, &minor, &caplow, &caphigh); if (!NT_STATUS_IS_OK(status)) { @@ -4012,6 +4079,12 @@ static int cmd_stat(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"stat\" command can be used.\n"); + return 1; + } + status = cli_posix_stat(targetcli, targetname, &sbuf); if (!NT_STATUS_IS_OK(status)) { d_printf("%s stat file %s\n", @@ -4126,6 +4199,12 @@ static int cmd_chown(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"chown\" command can be used.\n"); + return 1; + } + status = cli_posix_chown(targetcli, targetname, uid, gid); if (!NT_STATUS_IS_OK(status)) { d_printf("%s chown file %s uid=%d, gid=%d\n", -- 2.25.1 From bfcf165b29b30dd1f8037ab0f9a9e03731d2642f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 14:44:05 -0800 Subject: [PATCH 83/99] CVE-2021-44141: s4: torture: In raw.search:test_one_file() add a second connection. Change from torture_suite_add_1smb_test() to torture_suite_add_2smb_test(). Not yet used. We will need this to do SMB1+POSIX search calls on a connection on which we have negotiated SMB1+POSIX. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source4/torture/raw/search.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index 9b140928689..f6ad569dd4b 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -300,8 +300,9 @@ static union smb_search_data *find(const char *name) /* basic testing of all RAW_SEARCH_* calls using a single file */ -static bool test_one_file(struct torture_context *tctx, - struct smbcli_state *cli) +static bool test_one_file(struct torture_context *tctx, + struct smbcli_state *cli, + struct smbcli_state *cli_unix) { bool ret = true; int fnum; @@ -1571,7 +1572,7 @@ struct torture_suite *torture_raw_search(TALLOC_CTX *mem_ctx) { struct torture_suite *suite = torture_suite_create(mem_ctx, "search"); - torture_suite_add_1smb_test(suite, "one file search", test_one_file); + torture_suite_add_2smb_test(suite, "one file search", test_one_file); torture_suite_add_1smb_test(suite, "many files", test_many_files); torture_suite_add_1smb_test(suite, "sorted", test_sorted); torture_suite_add_1smb_test(suite, "modify search", test_modify_search); -- 2.25.1 From 08c40af638154fa009e6b6f526a357b10ba7e3ba Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 14:48:20 -0800 Subject: [PATCH 84/99] CVE-2021-44141: s4: torture: raw.search: Add setup_smb1_posix(). Call it on the second connection in test_one_file(). Not yet used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source4/torture/raw/search.c | 59 ++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index f6ad569dd4b..3f7de80ad0f 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -297,6 +297,55 @@ static union smb_search_data *find(const char *name) return NULL; } +/* + * Negotiate SMB1+POSIX. + */ + +static NTSTATUS setup_smb1_posix(struct torture_context *tctx, + struct smbcli_state *cli_unix) +{ + struct smb_trans2 tp; + uint16_t setup; + uint8_t data[12]; + uint8_t params[4]; + uint32_t cap = cli_unix->transport->negotiate.capabilities; + + if ((cap & CAP_UNIX) == 0) { + /* + * Server doesn't support SMB1+POSIX. + * The caller will skip the UNIX info + * level anyway. + */ + torture_comment(tctx, + "Server doesn't support SMB1+POSIX\n"); + return NT_STATUS_OK; + } + + /* Setup POSIX on this connection. */ + SSVAL(data, 0, CIFS_UNIX_MAJOR_VERSION); + SSVAL(data, 2, CIFS_UNIX_MINOR_VERSION); + SBVAL(data,4,((uint64_t)( + CIFS_UNIX_POSIX_ACLS_CAP| + CIFS_UNIX_POSIX_PATHNAMES_CAP| + CIFS_UNIX_FCNTL_LOCKS_CAP| + CIFS_UNIX_EXTATTR_CAP| + CIFS_UNIX_POSIX_PATH_OPERATIONS_CAP))); + setup = TRANSACT2_SETFSINFO; + tp.in.max_setup = 0; + tp.in.flags = 0; + tp.in.timeout = 0; + tp.in.setup_count = 1; + tp.in.max_param = 0; + tp.in.max_data = 0; + tp.in.setup = &setup; + tp.in.trans_name = NULL; + SSVAL(params, 0, 0); + SSVAL(params, 2, SMB_SET_CIFS_UNIX_INFO); + tp.in.params = data_blob_talloc(tctx, params, 4); + tp.in.data = data_blob_talloc(tctx, data, 12); + return smb_raw_trans2(cli_unix->tree, tctx, &tp); +} + /* basic testing of all RAW_SEARCH_* calls using a single file */ @@ -315,6 +364,16 @@ static bool test_one_file(struct torture_context *tctx, internal_info_supported; union smb_search_data *s; + status = setup_smb1_posix(tctx, cli_unix); + if (!NT_STATUS_IS_OK(status)) { + torture_result(tctx, + TORTURE_FAIL, + __location__"setup_smb1_posix() failed (%s)\n", + nt_errstr(status)); + ret = false; + goto done; + } + fnum = create_complex_file(cli, tctx, fname); if (fnum == -1) { torture_result(tctx, -- 2.25.1 From a7b6aa7d1f20dfb565605d662404d3988c83e5c8 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 14:51:39 -0800 Subject: [PATCH 85/99] CVE-2021-44141: s4: torture: Fix raw.search:test_one_file() by using the SMB1+POSIX connection for POSIX info levels. Remove the following entry in knownfail.d/posix_infolevel_fails. ^samba3.raw.search.one\ file\ search.* from knownfail.d/posix_infolevel_fails BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 1 - source4/torture/raw/search.c | 13 +++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails index 78a6781684c..4ce4a9cec91 100644 --- a/selftest/knownfail.d/posix_infolevel_fails +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -5,4 +5,3 @@ ^samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* ^samba3.unix.info2.info2\(nt4_dc_smb1\) ^samba3.unix.info2.info2\(ad_dc_smb1\) -^samba3.raw.search.one\ file\ search.* diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index 3f7de80ad0f..575bbd03fb7 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -389,10 +389,19 @@ static bool test_one_file(struct torture_context *tctx, for (i=0;itransport->negotiate.capabilities; + struct smbcli_state *cli_search = cli; torture_comment(tctx, "Testing %s\n", levels[i].name); - levels[i].status = torture_single_search(cli, tctx, fname, + if (levels[i].data_level == RAW_SEARCH_DATA_UNIX_INFO) { + /* + * For an SMB1+POSIX info level, use the cli_unix + * connection. + */ + cli_search = cli_unix; + } + + levels[i].status = torture_single_search(cli_search, tctx, fname, levels[i].level, levels[i].data_level, 0, @@ -416,7 +425,7 @@ static bool test_one_file(struct torture_context *tctx, continue; } - status = torture_single_search(cli, tctx, fname2, + status = torture_single_search(cli_search, tctx, fname2, levels[i].level, levels[i].data_level, 0, -- 2.25.1 From 300abd383ea7fc0b1b8c59d5a8c90201f216dcd6 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 12:15:06 -0800 Subject: [PATCH 86/99] CVE-2021-44141: s4: torture: Fix unix.info2 test to actually negotiate SMB1+POSIX before using POSIX calls. Cope with the minor difference in wildcard search return when we're actually using SMB1+POSIX on the server (SMB1+POSIX treats all directory search paths as wildcards). Remove the following entries in knownfail.d/posix_infolevel_fails. samba3.unix.info2.info2\(nt4_dc_smb1\) samba3.unix.info2.info2\(ad_dc_smb1\) BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 2 -- source4/torture/unix/unix_info2.c | 42 ++++++++++++++++++++-- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails index 4ce4a9cec91..3bfff110771 100644 --- a/selftest/knownfail.d/posix_infolevel_fails +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -3,5 +3,3 @@ ^samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* ^samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* ^samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* -^samba3.unix.info2.info2\(nt4_dc_smb1\) -^samba3.unix.info2.info2\(ad_dc_smb1\) diff --git a/source4/torture/unix/unix_info2.c b/source4/torture/unix/unix_info2.c index 6b275435551..2098b225e7f 100644 --- a/source4/torture/unix/unix_info2.c +++ b/source4/torture/unix/unix_info2.c @@ -19,6 +19,7 @@ #include "includes.h" #include "libcli/libcli.h" +#include "libcli/raw/raw_proto.h" #include "torture/util.h" #include "torture/unix/proto.h" #include "lib/cmdline/cmdline.h" @@ -53,6 +54,10 @@ static struct smbcli_state *connect_to_server(struct torture_context *tctx) const char *share = torture_setting_string(tctx, "share", NULL); struct smbcli_options options; struct smbcli_session_options session_options; + struct smb_trans2 tp; + uint16_t setup; + uint8_t data[12]; + uint8_t params[4]; lpcfg_smbcli_options(tctx->lp_ctx, &options); lpcfg_smbcli_session_options(tctx->lp_ctx, &session_options); @@ -72,6 +77,33 @@ static struct smbcli_state *connect_to_server(struct torture_context *tctx) return NULL; } + /* Setup POSIX on the server. */ + SSVAL(data, 0, CIFS_UNIX_MAJOR_VERSION); + SSVAL(data, 2, CIFS_UNIX_MINOR_VERSION); + SBVAL(data,4,((uint64_t)( + CIFS_UNIX_POSIX_ACLS_CAP| + CIFS_UNIX_POSIX_PATHNAMES_CAP| + CIFS_UNIX_FCNTL_LOCKS_CAP| + CIFS_UNIX_EXTATTR_CAP| + CIFS_UNIX_POSIX_PATH_OPERATIONS_CAP))); + setup = TRANSACT2_SETFSINFO; + tp.in.max_setup = 0; + tp.in.flags = 0; + tp.in.timeout = 0; + tp.in.setup_count = 1; + tp.in.max_param = 0; + tp.in.max_data = 0; + tp.in.setup = &setup; + tp.in.trans_name = NULL; + SSVAL(params, 0, 0); + SSVAL(params, 2, SMB_SET_CIFS_UNIX_INFO); + tp.in.params = data_blob_talloc(tctx, params, 4); + tp.in.data = data_blob_talloc(tctx, data, 12); + + status = smb_raw_trans2(cli->tree, tctx, &tp); + torture_assert_ntstatus_equal(tctx, status, NT_STATUS_OK, + "doing SMB_SET_CIFS_UNIX_INFO"); + return cli; } @@ -245,8 +277,14 @@ static bool find_single_info2(void *mem_ctx, torture_assert_int_equal(torture, search.t2ffirst.out.count, 1, "expected exactly one result"); - torture_assert_int_equal(torture, search.t2ffirst.out.end_of_search, 1, - "expected end_of_search to be true"); + /* + * In smbd directory listings using POSIX extensions + * always treat the search pathname as a wildcard, + * so don't expect end_of_search to be set here. Wildcard + * searches always need a findnext to end the search. + */ + torture_assert_int_equal(torture, search.t2ffirst.out.end_of_search, 0, + "expected end_of_search to be false"); return check_unix_info2(torture, info2); } -- 2.25.1 From a180e5726d598192e99ac4a26a2a3752bf7ac7c7 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 12:12:36 -0800 Subject: [PATCH 87/99] CVE-2021-44141: s3: tests: Fix the samba3.blackbox.inherit_owner test to actually negotiate SMB1+POSIX before using POSIX calls. Remove the following entry in knownfail.d/posix_infolevel_fails. samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 1 - source3/script/tests/test_inherit_owner.sh | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails index 3bfff110771..a865a2055b2 100644 --- a/selftest/knownfail.d/posix_infolevel_fails +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -2,4 +2,3 @@ ^samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* ^samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* ^samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* -^samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* diff --git a/source3/script/tests/test_inherit_owner.sh b/source3/script/tests/test_inherit_owner.sh index 7e1333787aa..9783235883c 100755 --- a/source3/script/tests/test_inherit_owner.sh +++ b/source3/script/tests/test_inherit_owner.sh @@ -79,7 +79,7 @@ unix_owner_id_is() { local fname=$2 local expected_id=$3 local actual_id - actual_id=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null | sed -rn 's/^# owner: (.*)/\1/p') + actual_id=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null | sed -rn 's/^# owner: (.*)/\1/p') if ! test "x$actual_id" = "x$expected_id" ; then echo "Actual uid of $share/$fname is [$actual_id] expected [$expected_id]" exit 1 -- 2.25.1 From c7aa173d2a44b3cf254b3739c7aedc2d5c8c0d58 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 00:05:35 -0800 Subject: [PATCH 88/99] CVE-2021-44141: s3: tests: Fix the samba3.blackbox.acl_xattr test to actually negotiate SMB1+POSIX before using POSIX calls. Remove the following entries in knownfail.d/posix_infolevel_fails. samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 3 --- source3/script/tests/test_acl_xattr.sh | 12 ++++++------ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails index a865a2055b2..bf8a884cb16 100644 --- a/selftest/knownfail.d/posix_infolevel_fails +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -1,4 +1 @@ ^samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) -^samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* -^samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* -^samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* diff --git a/source3/script/tests/test_acl_xattr.sh b/source3/script/tests/test_acl_xattr.sh index f134ff79c91..8abd7476244 100755 --- a/source3/script/tests/test_acl_xattr.sh +++ b/source3/script/tests/test_acl_xattr.sh @@ -55,9 +55,9 @@ nt_affects_posix() { local b4 local af local fname="$share.$$" - b4=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + b4=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -a "ACL:$SERVER\force_user:ALLOWED/0x0/READ" 2>/dev/null || exit 1 - af=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + af=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "before: $b4" echo "after: $af" echo "${b4}" | grep -q "^# owner:" || exit 1 @@ -90,12 +90,12 @@ nt_affects_chown() { #basic sanity... test "$b4_expected != $af_expected" || exit 1 - b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "${b4_actual}" | grep -q "^# owner:" || exit 1 b4_actual=$(echo "$b4_actual" | sed -rn 's/^# owner: (.*)/\1/p') $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -a "ACL:$SERVER\force_user:ALLOWED/0x0/FULL" || exit 1 $SMBCACLS //$SERVER/$share $fname -U force_user%$PASSWORD -C force_user 2>/dev/null || exit 1 - af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "${af_actual}" | grep -q "^# owner:" || exit 1 af_actual=$(echo "$af_actual" | sed -rn 's/^# owner: (.*)/\1/p') echo "before: $b4_actual" @@ -124,11 +124,11 @@ nt_affects_chgrp() { #basic sanity... test "$b4_expected" != "$af_expected" || exit 1 - b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "${b4_actual}" | grep -q "^# group:" || exit 1 b4_actual=$(echo "$b4_actual" | sed -rn 's/^# group: (.*)/\1/p') $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -G domadmins 2>/dev/null || exit 1 - af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "${af_actual}" | grep -q "^# group:" || exit 1 af_actual=$(echo "$af_actual" | sed -rn 's/^# group: (.*)/\1/p') echo "before: $b4_actual" -- 2.25.1 From 3e0d40f5481f2343fa93e204f2c432e1a2335c98 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 18 Nov 2021 12:16:44 -0800 Subject: [PATCH 89/99] CVE-2021-44141: s3: smbtorture3: Fix POSIX-BLOCKING-LOCK to actually negotiate SMB1+POSIX before using POSIX calls. This must be done before doing POSIX calls on a connection. Remove the final entry in knownfail.d/posix_infolevel_fails samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) And remove the file knownfail.d/posix_infolevel_fails itself. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 1 - source3/torture/torture.c | 5 +++++ 2 files changed, 5 insertions(+), 1 deletion(-) delete mode 100644 selftest/knownfail.d/posix_infolevel_fails diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails deleted file mode 100644 index bf8a884cb16..00000000000 --- a/selftest/knownfail.d/posix_infolevel_fails +++ /dev/null @@ -1 +0,0 @@ -^samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 0d3b01326b1..e3d26ddb261 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -8929,6 +8929,11 @@ static bool run_posix_blocking_lock(int dummy) return false; } + status = torture_setup_unix_extensions(cli2); + if (!NT_STATUS_IS_OK(status)) { + return false; + } + cli_setatr(cli1, fname, 0, 0); cli_posix_unlink(cli1, fname); -- 2.25.1 From 9e90f31639a71ba4c8099c9da4ad25102a36873b Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 12:28:54 -0800 Subject: [PATCH 90/99] CVE-2021-44141: s3: torture: Add samba3.blackbox.test_symlink_traversal.SMB2. Add to knownfail.d/symlink_traversal BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 1 + .../tests/test_symlink_traversal_smb2.sh | 263 ++++++++++++++++++ source3/selftest/tests.py | 6 + 3 files changed, 270 insertions(+) create mode 100644 selftest/knownfail.d/symlink_traversal create mode 100755 source3/script/tests/test_symlink_traversal_smb2.sh diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal new file mode 100644 index 00000000000..a8ac4bbae1f --- /dev/null +++ b/selftest/knownfail.d/symlink_traversal @@ -0,0 +1 @@ +^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) diff --git a/source3/script/tests/test_symlink_traversal_smb2.sh b/source3/script/tests/test_symlink_traversal_smb2.sh new file mode 100755 index 00000000000..7e1de6dde1a --- /dev/null +++ b/source3/script/tests/test_symlink_traversal_smb2.sh @@ -0,0 +1,263 @@ +#!/bin/sh + +if [ $# -lt 7 ]; then +cat < "$tmpfile" < Date: Tue, 7 Dec 2021 12:32:19 -0800 Subject: [PATCH 91/99] CVE-2021-44141: s3: torture: Add samba3.blackbox.test_symlink_traversal.SMB1. Add to knownfail.d/symlink_traversal. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 1 + .../tests/test_symlink_traversal_smb1.sh | 263 ++++++++++++++++++ source3/selftest/tests.py | 4 + 3 files changed, 268 insertions(+) create mode 100755 source3/script/tests/test_symlink_traversal_smb1.sh diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal index a8ac4bbae1f..2a51ff3f91d 100644 --- a/selftest/knownfail.d/symlink_traversal +++ b/selftest/knownfail.d/symlink_traversal @@ -1 +1,2 @@ ^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) +^samba3.blackbox.test_symlink_traversal.SMB1.symlink_traversal_SMB1\(fileserver_smb1_done\) diff --git a/source3/script/tests/test_symlink_traversal_smb1.sh b/source3/script/tests/test_symlink_traversal_smb1.sh new file mode 100755 index 00000000000..1deaaccbb54 --- /dev/null +++ b/source3/script/tests/test_symlink_traversal_smb1.sh @@ -0,0 +1,263 @@ +#!/bin/sh + +if [ $# -lt 7 ]; then +cat < "$tmpfile" < Date: Tue, 7 Dec 2021 12:34:38 -0800 Subject: [PATCH 92/99] CVE-2021-44141: s3: torture: Add samba3.blackbox.test_symlink_traversal.SMB1.posix Add to knownfail.d/symlink_traversal. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 1 + .../test_symlink_traversal_smb1_posix.sh | 270 ++++++++++++++++++ source3/selftest/tests.py | 5 + 3 files changed, 276 insertions(+) create mode 100755 source3/script/tests/test_symlink_traversal_smb1_posix.sh diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal index 2a51ff3f91d..25a4da8f250 100644 --- a/selftest/knownfail.d/symlink_traversal +++ b/selftest/knownfail.d/symlink_traversal @@ -1,2 +1,3 @@ ^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) ^samba3.blackbox.test_symlink_traversal.SMB1.symlink_traversal_SMB1\(fileserver_smb1_done\) +^samba3.blackbox.test_symlink_traversal.SMB1.posix.symlink_traversal_SMB1_posix\(fileserver_smb1_done\) diff --git a/source3/script/tests/test_symlink_traversal_smb1_posix.sh b/source3/script/tests/test_symlink_traversal_smb1_posix.sh new file mode 100755 index 00000000000..6241434dcf6 --- /dev/null +++ b/source3/script/tests/test_symlink_traversal_smb1_posix.sh @@ -0,0 +1,270 @@ +#!/bin/sh + +if [ $# -lt 7 ]; then +cat < "$tmpfile" < Date: Tue, 7 Dec 2021 12:56:51 -0800 Subject: [PATCH 93/99] CVE-2021-44141: s3: torture: In test_smbclient_s3, change the error codes expected for test_widelinks() and test_nosymlinks() from ACCESS_DENIED to NT_STATUS_OBJECT_NAME_NOT_FOUND. For SMB1/2/3 (minus posix) we need to treat bad symlinks as though they don't exist. Add to knwownfail.d/symlink_traversal BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 2 ++ selftest/target/Samba3.pm | 2 +- source3/script/tests/test_smbclient_s3.sh | 10 +++++----- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal index 25a4da8f250..840ab38b0f9 100644 --- a/selftest/knownfail.d/symlink_traversal +++ b/selftest/knownfail.d/symlink_traversal @@ -1,3 +1,5 @@ ^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) ^samba3.blackbox.test_symlink_traversal.SMB1.symlink_traversal_SMB1\(fileserver_smb1_done\) ^samba3.blackbox.test_symlink_traversal.SMB1.posix.symlink_traversal_SMB1_posix\(fileserver_smb1_done\) +^samba3.blackbox.smbclient_s3.*.Ensure\ widelinks\ are\ restricted\(.*\) +^samba3.blackbox.smbclient_s3.*.follow\ symlinks\ \=\ no\(.*\) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 84903b87d3e..b901fd2677a 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -2496,7 +2496,7 @@ sub provision($$) create_file_chmod("$widelinks_target", 0666) or return undef; ## - ## This link should get ACCESS_DENIED + ## This link should get an error ## symlink "$widelinks_target", "$widelinks_shrdir/source"; ## diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh index 89a17656159..e250d4dd106 100755 --- a/source3/script/tests/test_smbclient_s3.sh +++ b/source3/script/tests/test_smbclient_s3.sh @@ -1044,12 +1044,12 @@ EOF return 1 fi -# This should fail with NT_STATUS_ACCESS_DENIED - echo "$out" | grep 'NT_STATUS_ACCESS_DENIED' +# This should fail with NT_STATUS_OBJECT_NAME_NOT_FOUND + echo "$out" | grep 'NT_STATUS_OBJECT_NAME_NOT_FOUND' ret=$? if [ $ret != 0 ] ; then echo "$out" - echo "failed - should get NT_STATUS_ACCESS_DENIED listing \\widelinks_share\\source" + echo "failed - should get NT_STATUS_OBJECT_NAME_NOT_FOUND listing \\widelinks_share\\source" return 1 fi } @@ -1168,11 +1168,11 @@ EOF return 1 fi - echo "$out" | grep 'NT_STATUS_ACCESS_DENIED' + echo "$out" | grep 'NT_STATUS_OBJECT_NAME_NOT_FOUND' ret=$? if [ $ret -ne 0 ] ; then echo "$out" - echo "failed - should get NT_STATUS_ACCESS_DENIED getting \\nosymlinks\\source" + echo "failed - should get NT_STATUS_OBJECT_NAME_NOT_FOUND getting \\nosymlinks\\source" return 1 fi -- 2.25.1 From dbeef6bc732f05da5b35274cb0782a914e7392d7 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 17:56:35 -0800 Subject: [PATCH 94/99] CVE-2021-44141: s3: torture: Change expected error return for samba3.smbtorture_s3.plain.POSIX.smbtorture. Trying to open a symlink as a terminal component should return NT_STATUS_OBJECT_NAME_NOT_FOUND, not NT_STATUS_OBJECT_PATH_NOT_FOUND. Mark as knownfail.d/simple_posix_open until we fix the server. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/simple_posix_open | 1 + source3/torture/torture.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 selftest/knownfail.d/simple_posix_open diff --git a/selftest/knownfail.d/simple_posix_open b/selftest/knownfail.d/simple_posix_open new file mode 100644 index 00000000000..5fcbdbdc2c6 --- /dev/null +++ b/selftest/knownfail.d/simple_posix_open @@ -0,0 +1 @@ +^samba3.smbtorture_s3.plain.POSIX.smbtorture\(.*\) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index e3d26ddb261..c5da3836eac 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -7984,9 +7984,9 @@ static bool run_simple_posix_open_test(int dummy) goto out; } else { if (!check_both_error(__LINE__, status, ERRDOS, ERRbadpath, - NT_STATUS_OBJECT_PATH_NOT_FOUND)) { + NT_STATUS_OBJECT_NAME_NOT_FOUND)) { printf("POSIX open of %s should have failed " - "with NT_STATUS_OBJECT_PATH_NOT_FOUND, " + "with NT_STATUS_OBJECT_NAME_NOT_FOUND, " "failed with %s instead.\n", sname, nt_errstr(status)); goto out; -- 2.25.1 From b97f4a6519f64cbcea2b6baa33d853faf4bc24cb Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 11:44:09 -0800 Subject: [PATCH 95/99] CVE-2021-44141: s3: smbd: For SMB1+POSIX clients trying to open a symlink, always return NT_STATUS_OBJECT_NAME_NOT_FOUND. Matches the error return from openat_pathref_fsp(). NT_STATUS_OBJECT_PATH_NOT_FOUND is for a bad component in a path, not a bad terminal symlink. Remove knownfail.d/simple_posix_open, we now pass. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/simple_posix_open | 1 - source3/smbd/open.c | 13 ++++++------- 2 files changed, 6 insertions(+), 8 deletions(-) delete mode 100644 selftest/knownfail.d/simple_posix_open diff --git a/selftest/knownfail.d/simple_posix_open b/selftest/knownfail.d/simple_posix_open deleted file mode 100644 index 5fcbdbdc2c6..00000000000 --- a/selftest/knownfail.d/simple_posix_open +++ /dev/null @@ -1 +0,0 @@ -^samba3.smbtorture_s3.plain.POSIX.smbtorture\(.*\) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 5ed2c035318..5d2e2a1abf2 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -1447,12 +1447,10 @@ static NTSTATUS open_file(files_struct *fsp, * POSIX client that hit a symlink. We don't want to * return NT_STATUS_STOPPED_ON_SYMLINK to avoid handling * this special error code in all callers, so we map - * this to NT_STATUS_OBJECT_PATH_NOT_FOUND. Historically - * the lower level functions returned status code mapped - * from errno by map_nt_error_from_unix() where ELOOP is - * mapped to NT_STATUS_OBJECT_PATH_NOT_FOUND. + * this to NT_STATUS_OBJECT_NAME_NOT_FOUND to match + * openat_pathref_fsp(). */ - status = NT_STATUS_OBJECT_PATH_NOT_FOUND; + status = NT_STATUS_OBJECT_NAME_NOT_FOUND; } if (!NT_STATUS_IS_OK(status)) { DEBUG(3,("Error opening file %s (%s) (local_flags=%d) " @@ -1535,9 +1533,10 @@ static NTSTATUS open_file(files_struct *fsp, { /* * Don't allow stat opens on symlinks directly unless - * it's a POSIX open. + * it's a POSIX open. Match the return code from + * openat_pathref_fsp(). */ - return NT_STATUS_OBJECT_PATH_NOT_FOUND; + return NT_STATUS_OBJECT_NAME_NOT_FOUND; } if (!fsp->fsp_flags.is_pathref) { -- 2.25.1 From 66774e97e200d686be9c54739dc67ff0ed56af6f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 14:33:17 -0800 Subject: [PATCH 96/99] CVE-2021-44141: s3: smbd: Inside check_reduced_name() ensure we return the correct error codes when failing symlinks. NT_STATUS_OBJECT_PATH_NOT_FOUND for a path component failure. NT_STATUS_OBJECT_NAME_NOT_FOUND for a terminal component failure. Remove: samba3.blackbox.test_symlink_traversal.SMB1.posix samba3.blackbox.smbclient_s3.*.Ensure\ widelinks\ are\ restricted\(.*\) samba3.blackbox.smbclient_s3.*.follow\ symlinks\ \=\ no\(.*\) in knownfail.d/symlink_traversal as we now pass these. Only one more fix remaining to get rid of knownfail.d/symlink_traversal completely. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 3 --- source3/smbd/vfs.c | 18 ++++++++++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal index 840ab38b0f9..2a51ff3f91d 100644 --- a/selftest/knownfail.d/symlink_traversal +++ b/selftest/knownfail.d/symlink_traversal @@ -1,5 +1,2 @@ ^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) ^samba3.blackbox.test_symlink_traversal.SMB1.symlink_traversal_SMB1\(fileserver_smb1_done\) -^samba3.blackbox.test_symlink_traversal.SMB1.posix.symlink_traversal_SMB1_posix\(fileserver_smb1_done\) -^samba3.blackbox.smbclient_s3.*.Ensure\ widelinks\ are\ restricted\(.*\) -^samba3.blackbox.smbclient_s3.*.follow\ symlinks\ \=\ no\(.*\) diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c index d8b7b1283fb..a6022902c1d 100644 --- a/source3/smbd/vfs.c +++ b/source3/smbd/vfs.c @@ -1146,6 +1146,7 @@ NTSTATUS check_reduced_name(connection_struct *conn, bool allow_symlinks = true; const char *conn_rootdir; size_t rootdir_len; + bool parent_dir_checked = false; DBG_DEBUG("check_reduced_name [%s] [%s]\n", fname, conn->connectpath); @@ -1207,6 +1208,7 @@ NTSTATUS check_reduced_name(connection_struct *conn, if (resolved_name == NULL) { return NT_STATUS_NO_MEMORY; } + parent_dir_checked = true; } else { resolved_name = resolved_fname->base_name; } @@ -1256,7 +1258,13 @@ NTSTATUS check_reduced_name(connection_struct *conn, conn_rootdir, resolved_name); TALLOC_FREE(resolved_fname); - return NT_STATUS_ACCESS_DENIED; + if (parent_dir_checked) { + /* Part of a component path. */ + return NT_STATUS_OBJECT_PATH_NOT_FOUND; + } else { + /* End of a path. */ + return NT_STATUS_OBJECT_NAME_NOT_FOUND; + } } } @@ -1311,7 +1319,13 @@ NTSTATUS check_reduced_name(connection_struct *conn, p); TALLOC_FREE(resolved_fname); TALLOC_FREE(new_fname); - return NT_STATUS_ACCESS_DENIED; + if (parent_dir_checked) { + /* Part of a component path. */ + return NT_STATUS_OBJECT_PATH_NOT_FOUND; + } else { + /* End of a path. */ + return NT_STATUS_OBJECT_NAME_NOT_FOUND; + } } } -- 2.25.1 From 9371ace08e603c745be14d6131b7a7713b36e782 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 14:39:42 -0800 Subject: [PATCH 97/99] CVE-2021-44141: s3: smbd: Fix a subtle bug in the error returns from filename_convert(). If filename_convert() fails to convert the path, we never call check_name(). This means we can return an incorrect error code (NT_STATUS_ACCESS_DENIED) if we ran into a symlink that points outside the share to a non-readable directory. We need to make sure in this case we always call check_name(). Remove knownfail.d/symlink_traversal. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 2 -- source3/smbd/filename.c | 36 ++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) delete mode 100644 selftest/knownfail.d/symlink_traversal diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal deleted file mode 100644 index 2a51ff3f91d..00000000000 --- a/selftest/knownfail.d/symlink_traversal +++ /dev/null @@ -1,2 +0,0 @@ -^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) -^samba3.blackbox.test_symlink_traversal.SMB1.symlink_traversal_SMB1\(fileserver_smb1_done\) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 0d82085870c..56ebdd9f370 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -30,6 +30,9 @@ #include "smbd/smbd.h" #include "smbd/globals.h" +static NTSTATUS check_name(connection_struct *conn, + const struct smb_filename *smb_fname); + uint32_t ucf_flags_from_smb_request(struct smb_request *req) { uint32_t ucf_flags = 0; @@ -529,6 +532,39 @@ static NTSTATUS unix_convert_step_search_fail(struct uc_state *state) if (errno == EACCES) { if ((state->ucf_flags & UCF_PREP_CREATEFILE) == 0) { + /* + * Could be a symlink pointing to + * a directory outside the share + * to which we don't have access. + * If so, we need to know that here + * so we can return the correct error code. + * check_name() is never called if we + * error out of filename_convert(). + */ + int ret; + NTSTATUS status; + struct smb_filename dname = (struct smb_filename) { + .base_name = state->dirpath, + .twrp = state->smb_fname->twrp, + }; + + /* handle null paths */ + if ((dname.base_name == NULL) || + (dname.base_name[0] == '\0')) { + return NT_STATUS_ACCESS_DENIED; + } + ret = SMB_VFS_LSTAT(state->conn, &dname); + if (ret != 0) { + return NT_STATUS_ACCESS_DENIED; + } + if (!S_ISLNK(dname.st.st_ex_mode)) { + return NT_STATUS_ACCESS_DENIED; + } + status = check_name(state->conn, &dname); + if (!NT_STATUS_IS_OK(status)) { + /* We know this is an intermediate path. */ + return NT_STATUS_OBJECT_PATH_NOT_FOUND; + } return NT_STATUS_ACCESS_DENIED; } else { /* -- 2.25.1 From d46ffccc0780b9ef6b5a49e3e17b665345bd4362 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 22:15:46 -0800 Subject: [PATCH 98/99] CVE-2021-44141: s3: torture: Add a test samba3.blackbox.test_symlink_rename.SMB1.posix that shows we still leak target info across a SMB1+POSIX rename. Add a knownfail.d/posix_sylink_rename BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_sylink_rename | 1 + .../tests/test_symlink_rename_smb1_posix.sh | 186 ++++++++++++++++++ source3/selftest/tests.py | 5 + 3 files changed, 192 insertions(+) create mode 100644 selftest/knownfail.d/posix_sylink_rename create mode 100755 source3/script/tests/test_symlink_rename_smb1_posix.sh diff --git a/selftest/knownfail.d/posix_sylink_rename b/selftest/knownfail.d/posix_sylink_rename new file mode 100644 index 00000000000..9c3cc0a41ba --- /dev/null +++ b/selftest/knownfail.d/posix_sylink_rename @@ -0,0 +1 @@ +^samba3.blackbox.test_symlink_rename.SMB1.posix.symlink_rename_SMB1_posix\(fileserver_smb1_done\) diff --git a/source3/script/tests/test_symlink_rename_smb1_posix.sh b/source3/script/tests/test_symlink_rename_smb1_posix.sh new file mode 100755 index 00000000000..7d2e0037b8d --- /dev/null +++ b/source3/script/tests/test_symlink_rename_smb1_posix.sh @@ -0,0 +1,186 @@ +#!/bin/sh + +if [ $# -lt 7 ]; then +cat < "$tmpfile" < Date: Tue, 7 Dec 2021 22:19:29 -0800 Subject: [PATCH 99/99] CVE-2021-44141: s3: smbd: Inside rename_internals_fsp(), we must use vfs_stat() for existence, not SMB_VFS_STAT(). We need to take SMB1+POSIX into account here and do an LSTAT if it's a POSIX name. Remove knownfail.d/posix_sylink_rename BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_sylink_rename | 1 - source3/smbd/reply.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) delete mode 100644 selftest/knownfail.d/posix_sylink_rename diff --git a/selftest/knownfail.d/posix_sylink_rename b/selftest/knownfail.d/posix_sylink_rename deleted file mode 100644 index 9c3cc0a41ba..00000000000 --- a/selftest/knownfail.d/posix_sylink_rename +++ /dev/null @@ -1 +0,0 @@ -^samba3.blackbox.test_symlink_rename.SMB1.posix.symlink_rename_SMB1_posix\(fileserver_smb1_done\) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index cc95bfa3af0..69278a1da87 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7266,7 +7266,7 @@ NTSTATUS rename_internals_fsp(connection_struct *conn, goto out; } - dst_exists = SMB_VFS_STAT(conn, smb_fname_dst) == 0; + dst_exists = vfs_stat(conn, smb_fname_dst) == 0; if(!replace_if_exists && dst_exists) { DEBUG(3, ("rename_internals_fsp: dest exists doing rename " -- 2.25.1