Attachment 17008 Details for Bug 14914 – Possible fix

[patch] Possible fix

wip.patch (text/plain), 4.09 KB, created by Ralph Böhme on 2021-11-20 15:46:56 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralph Böhme

Created: 2021-11-20 15:46:56 UTC

Size: 4.09 KB

patch

obsolete

>From c1b67e90a03e22ec18fe56c28cb10d3d6663befb Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Sat, 20 Nov 2021 16:29:10 +0100
>Subject: [PATCH 1/2] CVE-XXX: WIP: libadouble: harden parsing code
>
>---
> source3/lib/adouble.c | 81 +++++++++++++++++++++++++++++++++++--------
> 1 file changed, 67 insertions(+), 14 deletions(-)
>
>diff --git a/source3/lib/adouble.c b/source3/lib/adouble.c
>index f809a445081d..71ba0ffe46dd 100644
>--- a/source3/lib/adouble.c
>+++ b/source3/lib/adouble.c
>@@ -839,6 +839,68 @@ static bool ad_unpack_xattrs(struct adouble *ad)
> 	return true;
> }
> 
>+/*
>+ * 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 (also see
>+ * comment above) anyway.
>+ */
>+static bool ad_entry_check_size(uint32_t eid,
>+				size_t bufsize,
>+				uint32_t off,
>+				uint32_t got_len)
>+{
>+	off_t expected_len[] = {
>+		[ADEID_DFORK] = -1,
>+		[ADEID_RFORK] = -1,
>+		[ADEID_NAME] = -1,
>+		[ADEID_COMMENT] = ADEDLEN_COMMENT,
>+		[ADEID_ICONBW] = -1,
>+		[ADEID_ICONCOL] = -1,
>+		[ADEID_FILEI] = ADEDLEN_FILEI,
>+		[ADEID_FILEDATESI] = ADEDLEN_FILEDATESI,
>+		[ADEID_FINDERI] = ADEDLEN_FINDERI,
>+		[ADEID_MACFILEI] = ADEDLEN_MACFILEI,
>+		[ADEID_PRODOSFILEI] = ADEDLEN_PRODOSFILEI,
>+		[ADEID_MSDOSFILEI] = ADEDLEN_MSDOSFILEI,
>+		[ADEID_SHORTNAME] = ADEDLEN_SHORTNAME,
>+		[ADEID_AFPFILEI] = ADEDLEN_AFPFILEI,
>+		[ADEID_DID] = ADEDLEN_DID,
>+		[ADEID_PRIVDEV] = ADEDLEN_PRIVDEV,
>+		[ADEID_PRIVINO] = ADEDLEN_PRIVINO,
>+		[ADEID_PRIVSYN] = ADEDLEN_PRIVSYN,
>+		[ADEID_PRIVID] = ADEDLEN_PRIVID
>+	};
>+
>+	if (eid >= ADEID_MAX) {
>+		return false;
>+	}
>+	if (expected_len[eid] == 0) {
>+		/*
>+		 * Shouldn't happen: implicitly initialized to zero because
>+		 * explicit initializer missing.
>+		 */
>+		return false;
>+	}
>+	if (expected_len[eid] == -1) {
>+		/* Unused or no limit */
>+		return true;
>+	}
>+	if (got_len < expected_len[eid]) {
>+		return false;
>+	}
>+	if (off + got_len < off) {
>+		return false;
>+	}
>+	if (off + got_len > bufsize) {
>+		return false;
>+	}
>+	return true;
>+}
>+
> /**
>  * Unpack an AppleDouble blob into a struct adoble
>  **/
>@@ -901,20 +963,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.33.1
>
>
>From a48149024e5069a9166d5a2a756b46d5a1f0c6e0 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/2] CVE-XXX: 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.
>---
> source3/smbd/trans2.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
>index 5f763d4ab4d1..c6b0d4dae7f5 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.33.1
>

Actions: View

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