The branch, master has been updated
via 4cc51f9 vfs_fruit: enhance handling of malformed AppleDouble files
from 05b61ea lib: tdb: Use sigaction when testing for robust mutexes.
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 4cc51f905cb5cd80d2e289a124c0fe1630d945b5
Author: Ralph Boehme <[email protected]>
Date: Mon Mar 2 18:15:06 2015 +0100
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 <[email protected]>
Reviewed-by: Volker Lendecke <[email protected]>
Autobuild-User(master): Ralph Böhme <[email protected]>
Autobuild-Date(master): Thu Mar 26 12:39:01 CET 2015 on sn-devel-104
-----------------------------------------------------------------------
Summary of changes:
source3/modules/vfs_fruit.c | 79 +++++++++++++++++++++++++++++++++++++++++----
1 file changed, 73 insertions(+), 6 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index fbee321..74ea8f8 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 than 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 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)) {
@@ -625,6 +640,48 @@ 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 (off + len < off) {
+ DEBUG(1, ("offset wrap in eid %d: off: %" PRIu32
+ ", len: %" PRIu32 "\n",
+ eid, off, len));
+ return false;
+
+ }
+ if (off + 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 +716,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 +778,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 +906,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;
--
Samba Shared Repository
