The branch, v4-16-test has been updated via 29355d0a2d4 VERSION: Bump version up to Samba 4.16.0rc3... via a4763bd9d87 VERSION: Disable GIT_SNAPSHOT for the 4.16.0rc2 release. via 4c3863633d3 WHATSNEW: Add release notes for Samba 4.16.0rc2. via c278515c492 s3/rpc_server: install elasticsearch_mappings.json via b88d24e33b2 CVE-2021-44141: s3: smbd: Inside rename_internals_fsp(), we must use vfs_stat() for existence, not SMB_VFS_STAT(). via 239e915b8f7 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 86157b3c7bf CVE-2021-44141: s3: smbd: Fix a subtle bug in the error returns from filename_convert(). via f4202a0bccd CVE-2021-44141: s3: smbd: Inside check_reduced_name() ensure we return the correct error codes when failing symlinks. via 4106af6d620 CVE-2021-44141: s3: smbd: For SMB1+POSIX clients trying to open a symlink, always return NT_STATUS_OBJECT_NAME_NOT_FOUND. via b8da8b72205 CVE-2021-44141: s3: torture: Change expected error return for samba3.smbtorture_s3.plain.POSIX.smbtorture. via c6d70dad3a2 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 ea20599ff17 CVE-2021-44141: s3: torture: Add samba3.blackbox.test_symlink_traversal.SMB1.posix via e6ccaced533 CVE-2021-44141: s3: torture: Add samba3.blackbox.test_symlink_traversal.SMB1. via 1dcd818303b CVE-2021-44141: s3: torture: Add samba3.blackbox.test_symlink_traversal.SMB2. via ef822984360 CVE-2021-44142: libadouble: harden parsing code via 03c6ba0054b CVE-2021-44142: libadouble: add basic cmocka tests via 39eb60d97a4 CVE-2021-44142: libadouble: harden ad_unpack_xattrs() via 36f847861bc CVE-2021-44142: smbd: add Netatalk xattr used by vfs_fruit to the list of private Samba xattrs via 9d7dd721b81 CVE-2021-44142: libadouble: add defines for icon lengths via e4f18bfaec8 CVE-2022-0336: s4/dsdb/samldb: Don't return early when an SPN is re-added to an object via eaede91afd6 CVE-2022-0336: pytest: Add a test for an SPN conflict with a re-added SPN from 4d3054261df blackbox.ndrdump: fix test_ndrdump_fuzzed_NULL_struct_ntlmssp_CHALLENGE_MESSAGE test
https://git.samba.org/?p=samba.git;a=shortlog;h=v4-16-test - Log ----------------------------------------------------------------- commit 29355d0a2d4e2b64a0cd1b8d16067f94f1594114 Author: Jule Anger <jan...@samba.org> Date: Mon Jan 31 12:56:33 2022 +0100 VERSION: Bump version up to Samba 4.16.0rc3... and re-enable GIT_SNAPSHOT. Signed-off-by: Jule Anger <jan...@samba.org> Autobuild-User(v4-16-test): Stefan Metzmacher <me...@samba.org> Autobuild-Date(v4-16-test): Mon Jan 31 15:26:29 UTC 2022 on sn-devel-184 commit a4763bd9d87f9efe93fa6d3ffc0ae9588663f8ef Author: Jule Anger <jan...@samba.org> Date: Mon Jan 31 12:56:06 2022 +0100 VERSION: Disable GIT_SNAPSHOT for the 4.16.0rc2 release. Signed-off-by: Jule Anger <jan...@samba.org> commit 4c3863633d31a3a45e5259e495c970e71df32732 Author: Jule Anger <jan...@samba.org> Date: Mon Jan 31 12:55:04 2022 +0100 WHATSNEW: Add release notes for Samba 4.16.0rc2. Signed-off-by: Jule Anger <jan...@samba.org> Signed-off-by: Stefan Metzmacher <me...@samba.org> commit c278515c492a1b9ca842e809120ecf3a1328d112 Author: Ralph Boehme <s...@samba.org> Date: Thu Jan 27 12:06:55 2022 +0100 s3/rpc_server: install elasticsearch_mappings.json This was removed accidentally remvoed by a7c65958a15149918415b7456d6f20ee8c9669d2 because the original code only installed the json file if the mdssvc was built as module: if bld.SAMBA3_IS_ENABLED_MODULE('rpc_mdssvc_module'): bld.INSTALL_FILES(bld.env.SAMBA_DATADIR, 'mdssvc/elasticsearch_mappings.json') Installing the json file should just depend on Elasticsearch support being enabled, regardless of the removed module support. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14961 Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> Autobuild-User(master): Noel Power <npo...@samba.org> Autobuild-Date(master): Fri Jan 28 10:22:31 UTC 2022 on sn-devel-184 (cherry picked from commit 0eecfddd071ea54844c56516dd7adc761be03c27) commit b88d24e33b2f4a2a540698520d76f1b8a2fe3e4d Author: Jeremy Allison <j...@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 <j...@samba.org> commit 239e915b8f721bab820ffba6ff355d828a34ffe9 Author: Jeremy Allison <j...@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 <j...@samba.org> commit 86157b3c7bfda64060ed4fbe23711aaa571be092 Author: Jeremy Allison <j...@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 <j...@samba.org> commit f4202a0bccd5d52aa5448748cdd2d67a68738fc0 Author: Jeremy Allison <j...@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 <j...@samba.org> commit 4106af6d620e4b4b66c015333d8e1dac9b4e9fd1 Author: Jeremy Allison <j...@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 <j...@samba.org> commit b8da8b722051030465e86aaf08305cb18cea02a3 Author: Jeremy Allison <j...@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 <j...@samba.org> commit c6d70dad3a28978e853e330ca4fcd96f435c83ac Author: Jeremy Allison <j...@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 <j...@samba.org> commit ea20599ff17315f9ee197634dabdfbb0e41b740c Author: Jeremy Allison <j...@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 <j...@samba.org> commit e6ccaced533d28f4685319b94dcf935dc7a5b150 Author: Jeremy Allison <j...@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 <j...@samba.org> commit 1dcd818303bdd5a1ea59faae114d290b51fc7999 Author: Jeremy Allison <j...@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 <j...@samba.org> commit ef8229843604bd2dcd17403a786ea70586e3d632 Author: Ralph Boehme <s...@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 <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 03c6ba0054b79143519fcf3079715139c0632a6f Author: Ralph Boehme <s...@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 <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 39eb60d97a4e2ae2765b5bc964e5606534ef664a Author: Ralph Boehme <s...@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 <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 36f847861bcea8cfca80121055685d7ddf6d73f2 Author: Ralph Boehme <s...@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 <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit 9d7dd721b81292c197f0c1556e38c006604b6727 Author: Ralph Boehme <s...@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 <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> commit e4f18bfaec844f261fa03616c9e55924366dfcf9 Author: Joseph Sutton <josephsut...@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 <josephsut...@catalyst.net.nz> Reviewed-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> commit eaede91afd6d171539aa5298644aa5fb107a6341 Author: Joseph Sutton <josephsut...@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 <josephsut...@catalyst.net.nz> Reviewed-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> ----------------------------------------------------------------------- Summary of changes: VERSION | 2 +- WHATSNEW.txt | 30 +- 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/rpc_server/wscript_build | 3 + 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 +- 23 files changed, 1629 insertions(+), 39 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/VERSION b/VERSION index 89dddc40217..c7035db0fb1 100644 --- a/VERSION +++ b/VERSION @@ -87,7 +87,7 @@ SAMBA_VERSION_PRE_RELEASE= # e.g. SAMBA_VERSION_RC_RELEASE=1 # # -> "3.0.0rc1" # ######################################################## -SAMBA_VERSION_RC_RELEASE=2 +SAMBA_VERSION_RC_RELEASE=3 ######################################################## # To mark SVN snapshots this should be set to 'yes' # diff --git a/WHATSNEW.txt b/WHATSNEW.txt index 71a8d9a103e..acf91910706 100644 --- a/WHATSNEW.txt +++ b/WHATSNEW.txt @@ -1,7 +1,7 @@ Release Announcements ===================== -This is the first release candidate of Samba 4.16. This is *not* +This is the second release candidate of Samba 4.16. This is *not* intended for production environments and is designed for testing purposes only. Please report any defects via the Samba bug reporting system at https://bugzilla.samba.org/. @@ -173,6 +173,34 @@ smb.conf changes rpc_server Removed rpc start on demand helpers Added true + +CHANGES SINCE 4.16.0rc1 +======================= + +o Jeremy Allison <j...@samba.org> + * BUG 14911: CVE-2021-44141: UNIX extensions in SMB1 disclose whether the + outside target of a symlink exists. + +o Ralph Boehme <s...@samba.org> + * BUG 14914: CVE-2021-44142: Out-of-Bound Read/Write on Samba vfs_fruit + module. + * BUG 14961: install elasticsearch_mappings.json + +o FeRD (Frank Dana) <ferd...@gmail.com> + * BUG 14947: samba-bgqd still notifying systemd, triggering log warnings + without NotifyAccess=all. + +o Stefan Metzmacher <me...@samba.org> + * BUG 14867: Printing no longer works on Windows 7 with 2021-10 monthly + rollup patch. + * BUG 14956: ndr_push_string() adds implicit termination for + STR_NOTERM|REMAINING empty strings. + +o Joseph Sutton <josephsut...@catalyst.net.nz> + * BUG 14950: CVE-2022-0336: Re-adding an SPN skips subsequent SPN conflict + checks. + + KNOWN ISSUES ============ 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 <s...@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. -- Samba Shared Repository