The Samba-Bugzilla – Attachment 17116 Details for
Bug 14914
CVE-2021-44142 [SECURITY] Out-of-Bound Read/Write on Samba vfs_fruit module
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
backport for 4.6.16
CVE-2021-44142-v4.6.16.patch (text/plain), 23.45 KB, created by
Noel Power
on 2022-01-23 20:19:07 UTC
(
hide
)
Description:
backport for 4.6.16
Filename:
MIME Type:
Creator:
Noel Power
Created:
2022-01-23 20:19:07 UTC
Size:
23.45 KB
patch
obsolete
>From 592aca7ac48947ff264ff2f24980a22863c644fb Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Thu, 13 Jan 2022 16:48:01 +0100 >Subject: [PATCH 1/6] 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@samba.org> >--- > source3/modules/vfs_fruit.c | 2 ++ > 1 file changed, 2 insertions(+) > >diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c >index 7ae51ee3f6f..56ba82142ed 100644 >--- a/source3/modules/vfs_fruit.c >+++ b/source3/modules/vfs_fruit.c >@@ -277,6 +277,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.31.1 > > >From 0c9e24ea2abb1882d74cf705dd4c692eb1705adb Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Sat, 20 Nov 2021 16:36:42 +0100 >Subject: [PATCH 2/6] 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@samba.org> >[slow@samba.org: conflict due to changed includes in source3/smbd/trans2.c] >--- > source3/smbd/trans2.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > >diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c >index ec7a5fa99d5..24f65a37044 100644 >--- a/source3/smbd/trans2.c >+++ b/source3/smbd/trans2.c >@@ -173,6 +173,16 @@ void aapl_force_zero_file_id(struct smbd_server_connection *sconn) > Refuse to allow clients to overwrite our private xattrs. > ****************************************************************************/ > >+/* >+ * Taken from vfs_fruit.c >+ */ >+#define NETATALK_META_XATTR "org.netatalk.Metadata" >+#if defined(HAVE_ATTROPEN) >+#define AFPINFO_EA_NETATALK NETATALK_META_XATTR >+#else >+#define AFPINFO_EA_NETATALK "user." NETATALK_META_XATTR >+#endif >+ > bool samba_private_attr_name(const char *unix_ea_name) > { > static const char * const prohibited_ea_names[] = { >@@ -180,6 +190,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.31.1 > > >From d9cfe712fed17e0f031e3955a04a712a12a31c26 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Fri, 26 Nov 2021 07:19:32 +0100 >Subject: [PATCH 3/6] 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@samba.org> >--- > source3/modules/vfs_fruit.c | 22 ++++++++++++++++++---- > 1 file changed, 18 insertions(+), 4 deletions(-) > >diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c >index 56ba82142ed..8fd2eb09bc7 100644 >--- a/source3/modules/vfs_fruit.c >+++ b/source3/modules/vfs_fruit.c >@@ -670,14 +670,27 @@ static bool ad_pack(struct adouble *ad) > 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; > >@@ -925,11 +938,12 @@ 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.31.1 > > >From d5f8a6f423f6bfba706d57459d78046920d61ce5 Mon Sep 17 00:00:00 2001 >From: Noel Power <noel.power@suse.com> >Date: Fri, 21 Jan 2022 14:52:53 +0000 >Subject: [PATCH 4/6] vfs_fruit: CVE-2021-44142 tweak buffer size check > >--- > source3/modules/vfs_fruit.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c >index 8fd2eb09bc7..7144b55083f 100644 >--- a/source3/modules/vfs_fruit.c >+++ b/source3/modules/vfs_fruit.c >@@ -687,7 +687,7 @@ static bool ad_unpack_xattrs(struct adouble *ad) > * 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) { >+ if (bufsize < AD_DATASZ_DOT_UND || bufsize > AD_XATTR_MAX_HDR_SIZE) { > return false; > } > >-- >2.31.1 > > >From ed9a8dcdb970962d0e9afbd52672e4295f3e31c6 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Thu, 25 Nov 2021 15:04:03 +0100 >Subject: [PATCH 5/6] CVE-2021-44142: libadouble: add basic cmocka tests > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14914 > >Signed-off-by: Ralph Boehme <slow@samba.org> >[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 | 386 +++++++++++++++++++ > source3/wscript_build | 5 + > 4 files changed, 396 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 eabe71401fc..277d9040855 100644 >--- a/selftest/tests.py >+++ b/selftest/tests.py >@@ -134,3 +134,5 @@ plantestsuite("wafsamba.duplicate_symbols", "none", [os.path.join(srcdir(), "bui > if with_cmocka: > plantestsuite("samba.unittests.krb5samba", "none", > [os.path.join(bindir(), "default/testsuite/unittests/test_krb5samba")]) >+ 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..1cebb875789 >--- /dev/null >+++ b/source3/lib/test_adouble.c >@@ -0,0 +1,386 @@ >+/* >+ * Unix SMB/CIFS implementation. >+ * >+ * Copyright (C) 2021 Ralph Boehme <slow@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 "vfs_fruit.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, >+ 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), >+ }; >+ >+ 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 92b95841bcf..cfd1b584e3e 100755 >--- a/source3/wscript_build >+++ b/source3/wscript_build >@@ -1093,6 +1093,11 @@ bld.SAMBA3_SUBSYSTEM('SPOOLSSD', > ''') > > ########################## BINARIES ################################# >+if bld.CONFIG_SET('HAVE_CMOCKA'): >+ bld.SAMBA3_BINARY('test_adouble', >+ source='lib/test_adouble.c', >+ deps='smbd_base STRING_REPLACE cmocka', >+ install=False) > > bld.SAMBA3_BINARY('smbd/smbd', > source='smbd/server.c smbd/smbd_cleanupd.c', >-- >2.31.1 > > >From 6dd0f863108cab92e97de2e4d283cd07a3c07caf Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Thu, 13 Jan 2022 17:03:02 +0100 >Subject: [PATCH 6/6] CVE-2021-44142: libadouble: harden parsing code > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14914 > >Signed-off-by: Ralph Boehme <slow@samba.org> >--- > selftest/knownfail.d/samba.unittests.adouble | 3 - > source3/modules/vfs_fruit.c | 116 ++++++++++++++++--- > 2 files changed, 101 insertions(+), 18 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/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c >index 7144b55083f..b7079892194 100644 >--- a/source3/modules/vfs_fruit.c >+++ b/source3/modules/vfs_fruit.c >@@ -483,6 +483,95 @@ static AfpInfo *afpinfo_new(TALLOC_CTX *ctx); > static ssize_t afpinfo_pack(const AfpInfo *ai, char *buf); > static AfpInfo *afpinfo_unpack(TALLOC_CTX *ctx, const void *data); > >+/* >+ * 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 > * >@@ -490,8 +579,15 @@ static AfpInfo *afpinfo_unpack(TALLOC_CTX *ctx, const void *data); > **/ > static 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; >@@ -555,7 +651,6 @@ static int ad_setdate(struct adouble *ad, unsigned int dateoff, uint32_t date) > return 0; > } > >- > /** > * Map on-disk AppleDouble id to enumerated id > **/ >@@ -875,20 +970,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.31.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
npower
:
review?
(
slow
)
Actions:
View
Attachments on
bug 14914
:
17008
|
17084
|
17087
|
17088
|
17089
|
17090
|
17091
|
17092
|
17093
|
17094
|
17115
| 17116 |
17117
|
17119
|
17128