[SCM] Samba Shared Repository - branch master updated

Stefan Metzmacher metze at samba.org
Mon Jan 31 16:27:02 UTC 2022


The branch, master has been updated
       via  e4e5539e402 CVE-2021-44141: s3: smbd: Inside rename_internals_fsp(), we must use vfs_stat() for existence, not SMB_VFS_STAT().
       via  a44435c6e76 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.
       via  be138920200 CVE-2021-44141: s3: smbd: Fix a subtle bug in the error returns from filename_convert().
       via  43455edd29a CVE-2021-44141: s3: smbd: Inside check_reduced_name() ensure we return the correct error codes when failing symlinks.
       via  458c7555a94 CVE-2021-44141: s3: smbd: For SMB1+POSIX clients trying to open a symlink, always return NT_STATUS_OBJECT_NAME_NOT_FOUND.
       via  f5b28d8aa33 CVE-2021-44141: s3: torture: Change expected error return for samba3.smbtorture_s3.plain.POSIX.smbtorture.
       via  3e9f6d704d3 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.
       via  4e75e24baab CVE-2021-44141: s3: torture: Add samba3.blackbox.test_symlink_traversal.SMB1.posix
       via  3bc85d615e6 CVE-2021-44141: s3: torture: Add samba3.blackbox.test_symlink_traversal.SMB1.
       via  1f7e870ddad CVE-2021-44141: s3: torture: Add samba3.blackbox.test_symlink_traversal.SMB2.
       via  751d7696646 CVE-2021-44142: libadouble: harden parsing code
       via  eb087934025 CVE-2021-44142: libadouble: add basic cmocka tests
       via  c61a06503ed CVE-2021-44142: libadouble: harden ad_unpack_xattrs()
       via  96083abc0c3 CVE-2021-44142: smbd: add Netatalk xattr used by vfs_fruit to the list of private Samba xattrs
       via  a9211cfe6e4 CVE-2021-44142: libadouble: add defines for icon lengths
       via  1a5dc817c0c CVE-2022-0336: s4/dsdb/samldb: Don't return early when an SPN is re-added to an object
       via  c58ede44f38 CVE-2022-0336: pytest: Add a test for an SPN conflict with a re-added SPN
      from  6063e8016fc s4:kdc: Translate HDB flags to SDB flags

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit e4e5539e402fd2116b4eb4f4f2d687da509491d6
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Dec 7 22:19:29 2021 -0800

    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 <jra at samba.org>
    
    Autobuild-User(master): Stefan Metzmacher <metze at samba.org>
    Autobuild-Date(master): Mon Jan 31 16:26:26 UTC 2022 on sn-devel-184

commit a44435c6e763e042e6c4cdbb70fc0479f1662c66
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Dec 7 22:15:46 2021 -0800

    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 <jra at samba.org>

commit be13892020013377994634a2367c3aff53245f44
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Dec 7 14:39:42 2021 -0800

    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 <jra at samba.org>

commit 43455edd29af00a0a4186f83557eec7481434170
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Dec 7 14:33:17 2021 -0800

    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 <jra at samba.org>

commit 458c7555a94f8d1e6e132b2568e900fddba3b2f9
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Dec 7 11:44:09 2021 -0800

    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 <jra at samba.org>

commit f5b28d8aa334f43fcc85fa70f897a2764287589f
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Dec 7 17:56:35 2021 -0800

    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 <jra at samba.org>

commit 3e9f6d704d3f3c51180cb2c5ee327e2a31106b52
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Dec 7 12:56:51 2021 -0800

    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 <jra at samba.org>

commit 4e75e24baab2773bbd1966ed38330118fbf51b26
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Dec 7 12:34:38 2021 -0800

    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 <jra at samba.org>

commit 3bc85d615e6415c7469d79730fdfb11a36d62932
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Dec 7 12:32:19 2021 -0800

    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 <jra at samba.org>

commit 1f7e870ddad935a04e0659b7f0c2f4caff3a0e9b
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Dec 7 12:28:54 2021 -0800

    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 <jra at samba.org>

commit 751d7696646975f34c1d053a90c49756e7a87c68
Author: Ralph Boehme <slow at samba.org>
Date:   Thu Jan 13 17:03:02 2022 +0100

    CVE-2021-44142: libadouble: harden parsing code
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14914
    
    Signed-off-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit eb087934025c5e1f29a584ea5594ac4838018cdf
Author: Ralph Boehme <slow at samba.org>
Date:   Thu Nov 25 15:04:03 2021 +0100

    CVE-2021-44142: libadouble: add basic cmocka tests
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14914
    
    Signed-off-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit c61a06503ede85658e9f0b3ac1f9c2a30ccf2fe7
Author: Ralph Boehme <slow at samba.org>
Date:   Fri Nov 26 07:19:32 2021 +0100

    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 <slow at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit 96083abc0c32aca397cfdf4341d179bfc5663d35
Author: Ralph Boehme <slow at samba.org>
Date:   Sat Nov 20 16:36:42 2021 +0100

    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 <slow at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit a9211cfe6e4bceb1f1e61c0ddfda25f3232aa411
Author: Ralph Boehme <slow at samba.org>
Date:   Thu Jan 13 16:48:01 2022 +0100

    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 <slow at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit 1a5dc817c0c9379bbaab14c676681b42b0039a3c
Author: Joseph Sutton <josephsutton at catalyst.net.nz>
Date:   Tue Jan 18 12:02:45 2022 +1300

    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 <josephsutton at catalyst.net.nz>
    Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>

commit c58ede44f382bd0125f761f0479c8d48156be400
Author: Joseph Sutton <josephsutton at catalyst.net.nz>
Date:   Tue Jan 18 11:56:38 2022 +1300

    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 <josephsutton at catalyst.net.nz>
    Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>

-----------------------------------------------------------------------

Summary of changes:
 python/samba/tests/ldap_spn.py                     |   7 +
 selftest/target/Samba3.pm                          |   2 +-
 selftest/tests.py                                  |   2 +
 source3/lib/adouble.c                              | 136 ++++++-
 source3/lib/adouble.h                              |   2 +
 source3/lib/test_adouble.c                         | 389 +++++++++++++++++++++
 source3/script/tests/test_smbclient_s3.sh          |  10 +-
 .../script/tests/test_symlink_rename_smb1_posix.sh | 186 ++++++++++
 .../script/tests/test_symlink_traversal_smb1.sh    | 263 ++++++++++++++
 .../tests/test_symlink_traversal_smb1_posix.sh     | 270 ++++++++++++++
 .../script/tests/test_symlink_traversal_smb2.sh    | 263 ++++++++++++++
 source3/selftest/tests.py                          |  20 ++
 source3/smbd/filename.c                            |  36 ++
 source3/smbd/open.c                                |  13 +-
 source3/smbd/reply.c                               |   2 +-
 source3/smbd/trans2.c                              |   2 +
 source3/smbd/vfs.c                                 |  18 +-
 source3/torture/torture.c                          |   4 +-
 source3/wscript_build                              |   5 +
 source4/dsdb/samdb/ldb_modules/samldb.c            |   3 +-
 20 files changed, 1596 insertions(+), 37 deletions(-)
 create mode 100644 source3/lib/test_adouble.c
 create mode 100755 source3/script/tests/test_symlink_rename_smb1_posix.sh
 create mode 100755 source3/script/tests/test_symlink_traversal_smb1.sh
 create mode 100755 source3/script/tests/test_symlink_traversal_smb1_posix.sh
 create mode 100755 source3/script/tests/test_symlink_traversal_smb2.sh


Changeset truncated at 500 lines:

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/target/Samba3.pm b/selftest/target/Samba3.pm
index 83941a85e15..7bb007c959d 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -2537,7 +2537,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/selftest/tests.py b/selftest/tests.py
index 1da18820c9d..e35c3fff3c1 100644
--- a/selftest/tests.py
+++ b/selftest/tests.py
@@ -445,3 +445,5 @@ plantestsuite("samba.unittests.credentials", "none",
               [os.path.join(bindir(), "default/auth/credentials/test_creds")])
 plantestsuite("samba.unittests.tsocket_bsd_addr", "none",
               [os.path.join(bindir(), "default/lib/tsocket/test_tsocket_bsd_addr")])
+plantestsuite("samba.unittests.adouble", "none",
+              [os.path.join(bindir(), "test_adouble")])
diff --git a/source3/lib/adouble.c b/source3/lib/adouble.c
index f809a445081..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;
@@ -707,14 +803,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;
 
@@ -901,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;
 		}
 
@@ -964,9 +1064,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;
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
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 <slow at samba.org>
+ *
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+#include "adouble.c"
+#include <cmocka.h>
+
+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,


-- 
Samba Shared Repository



More information about the samba-cvs mailing list