The Samba-Bugzilla – Attachment 17092 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]
Patch for 4.15 cherry-picked from master
CVE-2021-44142-v415.patch (text/plain), 21.92 KB, created by
Ralph Böhme
on 2022-01-16 15:38:39 UTC
(
hide
)
Description:
Patch for 4.15 cherry-picked from master
Filename:
MIME Type:
Creator:
Ralph Böhme
Created:
2022-01-16 15:38:39 UTC
Size:
21.92 KB
patch
obsolete
>From 7487b05bb925484a6f39a3e57841ec21331ad520 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/5] 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/lib/adouble.h | 2 ++ > 1 file changed, 2 insertions(+) > >diff --git a/source3/lib/adouble.h b/source3/lib/adouble.h >index 8b14d0ab871c..de44f3f5fdc7 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.34.1 > > >From f116fe730f05add81c4a4fb77dc0586fc61f27e5 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/5] 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> >--- > source3/smbd/trans2.c | 2 ++ > 1 file changed, 2 insertions(+) > >diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c >index a86ac3228e35..4f6d92955cfc 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.34.1 > > >From 08a1860b7cd496c324a29e5a868fe8d8c4020c94 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/5] 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/lib/adouble.c | 21 ++++++++++++++++++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > >diff --git a/source3/lib/adouble.c b/source3/lib/adouble.c >index f809a445081d..6cbe8a5aeda7 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.34.1 > > >From 07bfe2c983366babc122bed2b0b16a06704bd7b0 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Thu, 25 Nov 2021 15:04:03 +0100 >Subject: [PATCH 4/5] 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 | 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 000000000000..8b0314f2faec >--- /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 e7338985cafd..c87b41c1a665 100644 >--- a/selftest/tests.py >+++ b/selftest/tests.py >@@ -434,3 +434,5 @@ plantestsuite("samba.unittests.test_oLschema2ldif", "none", > [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 000000000000..615c22469c91 >--- /dev/null >+++ b/source3/lib/test_adouble.c >@@ -0,0 +1,389 @@ >+/* >+ * 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 "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, >+ 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 69febb537501..9df9bdd35b7b 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.34.1 > > >From e3519aa667be3c018c933ca2a37ee59bbb5098a2 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Thu, 13 Jan 2022 17:03:02 +0100 >Subject: [PATCH 5/5] 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/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 8b0314f2faec..000000000000 >--- 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 6cbe8a5aeda7..37fb686f17bb 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.34.1 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=17092">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
jra
:
review+
slow
:
ci-passed+
Actions:
View
Attachments on
bug 14914
:
17008
|
17084
|
17087
|
17088
|
17089
|
17090
|
17091
| 17092 |
17093
|
17094
|
17115
|
17116
|
17117
|
17119
|
17128