Attachment 17115 Details for Bug 14914 – backport for 4.11.14

[patch] backport for 4.11.14

CVE-2021-44142-v4.11.14.patch (text/plain), 22.62 KB, created by Noel Power on 2022-01-23 20:16:43 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Noel Power

Created: 2022-01-23 20:16:43 UTC

Size: 22.62 KB

patch

obsolete

>From 0608b65e3a914cbcc9995e0fdf204e8932f8ddf7 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/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 c516168633a..a89b6f7e5f1 100644
>--- a/source3/modules/vfs_fruit.c
>+++ b/source3/modules/vfs_fruit.c
>@@ -283,6 +283,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 1f45f1e0ac3324ddc51cd1b8afdade76154ac107 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>
>[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 78dabc016ed..232c984d014 100644
>--- a/source3/smbd/trans2.c
>+++ b/source3/smbd/trans2.c
>@@ -222,6 +222,16 @@ uint64_t smb_roundup(connection_struct *conn, uint64_t val)
>  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[] = {
>@@ -229,6 +239,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 7dce25cbe4ceffa90f895d916734f632a88565a4 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/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 a89b6f7e5f1..64d24bfce3f 100644
>--- a/source3/modules/vfs_fruit.c
>+++ b/source3/modules/vfs_fruit.c
>@@ -728,14 +728,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;
> 
>@@ -985,11 +998,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 fa562adbe44f9af5aa32dd988168b4d2fea08e9e 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                   | 393 +++++++++++++++++++
> source3/wscript_build                        |   5 +
> 4 files changed, 403 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 7589566ec14..34e506b3ac3 100644
>--- a/selftest/tests.py
>+++ b/selftest/tests.py
>@@ -401,3 +401,5 @@ plantestsuite("samba.unittests.test_registry_regfio", "none",
>               [os.path.join(bindir(), "default/source3/test_registry_regfio")])
> plantestsuite("samba.unittests.test_oLschema2ldif", "none",
>               [os.path.join(bindir(), "default/source4/utils/oLschema2ldif/test_oLschema2ldif")])
>+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..667d2a7542e
>--- /dev/null
>+++ b/source3/lib/test_adouble.c
>@@ -0,0 +1,393 @@
>+/*
>+ * 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 "includes.h"
>+extern NTSTATUS vfs_fruit_init(TALLOC_CTX *mem_ctx);
>+
>+#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),
>+	};
>+
>+	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 aa3c7175202..9fc19781099 100644
>--- a/source3/wscript_build
>+++ b/source3/wscript_build
>@@ -1083,6 +1083,11 @@ bld.SAMBA3_SUBSYSTEM('tevent-glib-glue',
> 
> ########################## BINARIES #################################
> 
>+bld.SAMBA3_BINARY('test_adouble',
>+                 source='lib/test_adouble.c',
>+                 deps='smbd_base STRING_REPLACE cmocka HASH_INODE',
>+                 install=False)
>+
> bld.SAMBA3_BINARY('smbd/smbd',
>                  source='smbd/server.c smbd/smbd_cleanupd.c',
>                  deps='''
>-- 
>2.31.1
>
>
>From 5542f3c754dc7c2e3d9fb49dff9ee3883938b29e 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/modules/vfs_fruit.c                  | 114 ++++++++++++++++---
> 2 files changed, 100 insertions(+), 17 deletions(-)
> delete mode 100644 selftest/knownfail.d/samba.unittests.adouble
>
>diff --git a/selftest/knownfail.d/samba.unittests.adouble b/selftest/knownfail.d/samba.unittests.adouble
>deleted file mode 100644
>index 8b0314f2fae..00000000000
>--- a/selftest/knownfail.d/samba.unittests.adouble
>+++ /dev/null
>@@ -1,3 +0,0 @@
>-^samba.unittests.adouble.parse_abouble_finderinfo2\(none\)
>-^samba.unittests.adouble.parse_abouble_finderinfo3\(none\)
>-^samba.unittests.adouble.parse_abouble_date2\(none\)
>diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
>index 64d24bfce3f..77fe38cedb1 100644
>--- a/source3/modules/vfs_fruit.c
>+++ b/source3/modules/vfs_fruit.c
>@@ -540,6 +540,94 @@ 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
>@@ -548,8 +636,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;
>@@ -935,20 +1030,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
>

Flags:

slow: review+

Actions: View

Attachments on bug 14914: 17008 | 17084 | 17087 | 17088 | 17089 | 17090 | 17091 | 17092 | 17093 | 17094 | 17115 | 17116 | 17117 | 17119 | 17128