From 10743111800015be14deed39b86515363b74b620 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Mon, 2 Mar 2015 18:15:06 +0100
Subject: [PATCH] vfs_fruit: enhance handling of malformed AppleDouble files

Trying for fixup a broken AppleDouble file with a resourcefork entry
offset + length > filesystem resulted in a crashing memmove() in
ad_convert().

Add a specific safety check that stats the ._ file and limits the
resource fork length to the filesize.

While we're at it, now that we know the filesize in ad_unpack(), add
additional checks that verify this.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11125

Signed-off-by: Ralph Boehme <slow@samba.org>
---
 source3/modules/vfs_fruit.c | 72 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 66 insertions(+), 6 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index fbee321..2af92ca 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -569,7 +569,7 @@ static bool ad_pack(struct adouble *ad)
 /**
  * Unpack an AppleDouble blob into a struct adoble
  **/
-static bool ad_unpack(struct adouble *ad, const int nentries)
+static bool ad_unpack(struct adouble *ad, const int nentries, size_t filesize)
 {
 	size_t bufsize = talloc_get_size(ad->ad_data);
 	int adentries, i;
@@ -612,11 +612,26 @@ static bool ad_unpack(struct adouble *ad, const int nentries)
 			return false;
 		}
 
+		/*
+		 * All entries other then the resource fork are
+		 * expected to be read into the ad_data buffer, so
+		 * ensure the specified offset is within that bound
+		 */
 		if ((off > bufsize) && (eid != ADEID_RFORK)) {
 			DEBUG(1, ("bogus eid %d: off: %" PRIu32 ", len: %" PRIu32 "\n",
 				  eid, off, len));
 			return false;
 		}
+
+		/*
+		 * All entries besides FiderInfo 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)) {
@@ -625,6 +640,41 @@ static bool ad_unpack(struct adouble *ad, const int nentries)
 			return false;
 		}
 
+		/*
+		 * That would be obviously broken
+		 */
+		if (off > filesize) {
+			DEBUG(1, ("bogus eid %d: off: %" PRIu32 ", len: %" PRIu32 "\n",
+				  eid, off, len));
+			return false;
+		}
+
+		/*
+		 * Check for any entry that has its end beyond the
+		 * filesize.
+		 */
+		if ((size_t)off + (size_t)len > filesize) {
+			/*
+			 * If this is the resource fork entry, we fix
+			 * up the length, for any other entry we bail
+			 * out.
+			 */
+			if (eid != ADEID_RFORK) {
+				DEBUG(1, ("bogus eid %d: off: %" PRIu32
+					  ", len: %" PRIu32 "\n",
+					  eid, off, len));
+				return false;
+			}
+
+			/*
+			 * Fixup the resource fork entry by limiting
+			 * the size to entryoffset - filesize.
+			 */
+			len = filesize - off;
+			DEBUG(1, ("Limiting ADEID_RFORK: off: %" PRIu32
+				  ", len: %" PRIu32 "\n", off, len));
+		}
+
 		ad->ad_eid[eid].ade_off = off;
 		ad->ad_eid[eid].ade_len = len;
 	}
@@ -659,9 +709,11 @@ static int ad_convert(struct adouble *ad, int fd)
 		goto exit;
 	}
 
-	memmove(map + ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI,
-		map + ad_getentryoff(ad, ADEID_RFORK),
-		ad_getentrylen(ad, ADEID_RFORK));
+	if (ad_getentrylen(ad, ADEID_RFORK) > 0) {
+		memmove(map + ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI,
+			map + ad_getentryoff(ad, ADEID_RFORK),
+			ad_getentrylen(ad, ADEID_RFORK));
+	}
 
 	ad_setentrylen(ad, ADEID_FINDERI, ADEDLEN_FINDERI);
 	ad_setentryoff(ad, ADEID_RFORK,
@@ -719,7 +771,7 @@ static ssize_t ad_header_read_meta(struct adouble *ad, const char *path)
 	}
 
 	/* Now parse entries */
-	ok = ad_unpack(ad, ADEID_NUM_XATTR);
+	ok = ad_unpack(ad, ADEID_NUM_XATTR, AD_DATASZ_XATTR);
 	if (!ok) {
 		DEBUG(2, ("invalid AppleDouble metadata xattr\n"));
 		errno = EINVAL;
@@ -847,8 +899,16 @@ static ssize_t ad_header_read_rsrc(struct adouble *ad, const char *path)
 			goto exit;
 		}
 
+		/* FIXME: direct sys_fstat(), we don't have an fsp */
+		rc = sys_fstat(fd, &sbuf,
+			       lp_fake_directory_create_times(
+				       SNUM(ad->ad_handle->conn)));
+		if (rc != 0) {
+			goto exit;
+		}
+
 		/* Now parse entries */
-		ok = ad_unpack(ad, ADEID_NUM_DOT_UND);
+		ok = ad_unpack(ad, ADEID_NUM_DOT_UND, sbuf.st_ex_size);
 		if (!ok) {
 			DEBUG(1, ("invalid AppleDouble ressource %s\n", path));
 			errno = EINVAL;
-- 
2.1.0