The Samba-Bugzilla – Attachment 17117 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]
backport for 4.4.2
CVE-2021-44142-v442.patch (text/plain), 15.58 KB, created by
Noel Power
on 2022-01-23 20:20:41 UTC
(
hide
)
Description:
backport for 4.4.2
Filename:
MIME Type:
Creator:
Noel Power
Created:
2022-01-23 20:20:41 UTC
Size:
15.58 KB
patch
obsolete
>From 1bb482b9e03521057db77aa0923a69463ca2901e Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Fri, 21 Jan 2022 17:52:52 +0000 >Subject: [PATCH 1/5] vfs_fruit: replace unsafe ad_entry macro with a function > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=12427 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Uri Simchoni <uri@samba.org> >(cherry picked from commit 3d5bf4b85f3ca120206a12b3d102aef2ead33082) >--- > source3/modules/vfs_fruit.c | 105 +++++++++++++++++++++++++++++++++++--------- > 1 file changed, 85 insertions(+), 20 deletions(-) > >diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c >index cb0d2848fa3..01269bd8f74 100644 >--- a/source3/modules/vfs_fruit.c >+++ b/source3/modules/vfs_fruit.c >@@ -342,7 +342,6 @@ typedef enum {ADOUBLE_META, ADOUBLE_RSRC} adouble_type_t; > #define ad_getentryoff(ad,eid) ((ad)->ad_eid[(eid)].ade_off) > #define ad_setentrylen(ad,eid,len) ((ad)->ad_eid[(eid)].ade_len = (len)) > #define ad_setentryoff(ad,eid,off) ((ad)->ad_eid[(eid)].ade_off = (off)) >-#define ad_entry(ad,eid) ((ad)->ad_data + ad_getentryoff((ad),(eid))) > > struct ad_entry { > size_t ade_off; >@@ -413,6 +412,23 @@ static int ad_write(struct adouble *ad, const char *path); > static int adouble_path(TALLOC_CTX *ctx, const char *path_in, char **path_out); > > /** >+ * Return a pointer to an AppleDouble entry >+ * >+ * Returns NULL if the entry is not present >+ **/ >+static char *ad_get_entry(const struct adouble *ad, int eid) >+{ >+ off_t off = ad_getentryoff(ad, eid); >+ size_t len = ad_getentrylen(ad, eid); >+ >+ if (off == 0 || len == 0) { >+ return NULL; >+ } >+ >+ return ad->ad_data + off; >+} >+ >+/** > * Get a date > **/ > static int ad_getdate(const struct adouble *ad, >@@ -420,18 +436,19 @@ static int ad_getdate(const struct adouble *ad, > uint32_t *date) > { > bool xlate = (dateoff & AD_DATE_UNIX); >+ char *p = NULL; > > dateoff &= AD_DATE_MASK; >- if (!ad_getentryoff(ad, ADEID_FILEDATESI)) { >+ p = ad_get_entry(ad, ADEID_FILEDATESI); >+ if (p == NULL) { > return -1; > } > > if (dateoff > AD_DATE_ACCESS) { > return -1; > } >- memcpy(date, >- ad_entry(ad, ADEID_FILEDATESI) + dateoff, >- sizeof(uint32_t)); >+ >+ memcpy(date, p + dateoff, sizeof(uint32_t)); > > if (xlate) { > *date = AD_DATE_TO_UNIX(*date); >@@ -445,9 +462,11 @@ static int ad_getdate(const struct adouble *ad, > static int ad_setdate(struct adouble *ad, unsigned int dateoff, uint32_t date) > { > bool xlate = (dateoff & AD_DATE_UNIX); >+ char *p = NULL; > >- if (!ad_getentryoff(ad, ADEID_FILEDATESI)) { >- return 0; >+ p = ad_get_entry(ad, ADEID_FILEDATESI); >+ if (p == NULL) { >+ return -1; > } > > dateoff &= AD_DATE_MASK; >@@ -459,7 +478,7 @@ static int ad_setdate(struct adouble *ad, unsigned int dateoff, uint32_t date) > return -1; > } > >- memcpy(ad_entry(ad, ADEID_FILEDATESI) + dateoff, &date, sizeof(date)); >+ memcpy(p + dateoff, &date, sizeof(date)); > > return 0; > } >@@ -943,6 +962,9 @@ static ssize_t ad_header_read_rsrc(struct adouble *ad, const char *path) > > if ((mode == O_RDWR) > && (ad_getentrylen(ad, ADEID_FINDERI) > ADEDLEN_FINDERI)) { >+ char *p_ad = NULL; >+ char *p_meta_ad = NULL; >+ > rc = ad_convert(ad, fd); > if (rc != 0) { > rc = -1; >@@ -972,9 +994,18 @@ static ssize_t ad_header_read_rsrc(struct adouble *ad, const char *path) > goto exit; > } > >- memcpy(ad_entry(meta_ad, ADEID_FINDERI), >- ad_entry(ad, ADEID_FINDERI), >- ADEDLEN_FINDERI); >+ p_ad = ad_get_entry(ad, ADEID_FINDERI); >+ if (p_ad == NULL) { >+ rc = -1; >+ goto exit; >+ } >+ p_meta_ad = ad_get_entry(meta_ad, ADEID_FINDERI); >+ if (p_meta_ad == NULL) { >+ rc = -1; >+ goto exit; >+ } >+ >+ memcpy(p_meta_ad, p_ad, ADEDLEN_FINDERI); > > rc = ad_write(meta_ad, path); > if (rc != 0) { >@@ -1559,8 +1590,15 @@ static bool empty_finderinfo(const struct adouble *ad) > { > > char emptybuf[ADEDLEN_FINDERI] = {0}; >+ char *fi = NULL; >+ >+ fi = ad_get_entry(ad, ADEID_FINDERI); >+ if (fi == NULL) { >+ DBG_ERR("Missing FinderInfo in struct adouble [%p]\n", ad); >+ return false; >+ } > if (memcmp(emptybuf, >- ad_entry(ad, ADEID_FINDERI), >+ fi, > ADEDLEN_FINDERI) == 0) { > return true; > } >@@ -1973,23 +2011,32 @@ static NTSTATUS readdir_attr_macmeta(struct vfs_handle_struct *handle, > ad = ad_get(talloc_tos(), handle, smb_fname->base_name, > ADOUBLE_META); > if (ad) { >+ char *p = ad_get_entry(ad, ADEID_FINDERI); >+ >+ if (p == NULL) { >+ DBG_ERR("No ADEID_FINDERI for [%s]\n", >+ smb_fname->base_name); >+ status = NT_STATUS_INVALID_PARAMETER; >+ goto out; >+ } >+ > if (S_ISREG(smb_fname->st.st_ex_mode)) { > /* finder_type */ > memcpy(&attr_data->attr_data.aapl.finder_info[0], >- ad_entry(ad, ADEID_FINDERI), 4); >+ p, 4); > > /* finder_creator */ > memcpy(&attr_data->attr_data.aapl.finder_info[0] + 4, >- ad_entry(ad, ADEID_FINDERI) + 4, 4); >+ p + 4, 4); > } > > /* finder_flags */ > memcpy(&attr_data->attr_data.aapl.finder_info[0] + 8, >- ad_entry(ad, ADEID_FINDERI) + 8, 2); >+ p + 8, 2); > > /* finder_ext_flags */ > memcpy(&attr_data->attr_data.aapl.finder_info[0] + 10, >- ad_entry(ad, ADEID_FINDERI) + 24, 2); >+ p + 24, 2); > > /* creation date */ > date_added = convert_time_t_to_uint32_t( >@@ -2000,6 +2047,7 @@ static NTSTATUS readdir_attr_macmeta(struct vfs_handle_struct *handle, > } > } > >+out: > TALLOC_FREE(ad); > return status; > } >@@ -2679,6 +2727,7 @@ static ssize_t fruit_pread(vfs_handle_struct *handle, > > if (ad->ad_type == ADOUBLE_META) { > char afpinfo_buf[AFP_INFO_SIZE]; >+ char *p = NULL; > size_t to_return; > > /* >@@ -2707,9 +2756,16 @@ static ssize_t fruit_pread(vfs_handle_struct *handle, > goto exit; > } > >- memcpy(&ai->afpi_FinderInfo[0], >- ad_entry(ad, ADEID_FINDERI), >- ADEDLEN_FINDERI); >+ p = ad_get_entry(ad, ADEID_FINDERI); >+ if (p == NULL) { >+ DBG_ERR("No ADEID_FINDERI for [%s]\n", >+ fsp->fsp_name->base_name); >+ rc = -1; >+ goto exit; >+ } >+ >+ memcpy(&ai->afpi_FinderInfo[0], p, ADEDLEN_FINDERI); >+ > len = afpinfo_pack(ai, afpinfo_buf); > if (len != AFP_INFO_SIZE) { > rc = -1; >@@ -2797,6 +2853,7 @@ static ssize_t fruit_pwrite(vfs_handle_struct *handle, > } > > if (ad->ad_type == ADOUBLE_META) { >+ char * p = NULL; > if (n != AFP_INFO_SIZE || offset != 0) { > DEBUG(1, ("unexpected offset=%jd or size=%jd\n", > (intmax_t)offset, (intmax_t)n)); >@@ -2808,7 +2865,15 @@ static ssize_t fruit_pwrite(vfs_handle_struct *handle, > rc = -1; > goto exit; > } >- memcpy(ad_entry(ad, ADEID_FINDERI), >+ p = ad_get_entry(ad, ADEID_FINDERI); >+ if (p == NULL) { >+ DBG_ERR("No ADEID_FINDERI for [%s]\n", >+ fsp->fsp_name->base_name); >+ rc = -1; >+ goto exit; >+ } >+ >+ memcpy(p, > &ai->afpi_FinderInfo[0], ADEDLEN_FINDERI); > if (empty_finderinfo(ad)) { > /* Discard metadata */ >-- >2.12.3 > > >From 306869688b42cdaf59e0a5474d033d73a04e2950 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Thu, 13 Jan 2022 16:48:01 +0100 >Subject: [PATCH 2/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 01269bd8f74..4148e86b5df 100644 >--- a/source3/modules/vfs_fruit.c >+++ b/source3/modules/vfs_fruit.c >@@ -258,6 +258,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.12.3 > > >From 8d03fc781354e7abb0361afbc5ea4d6f26a1d12e Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Sat, 20 Nov 2021 16:36:42 +0100 >Subject: [PATCH 3/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 b65f5814fab..698689750d4 100644 >--- a/source3/smbd/trans2.c >+++ b/source3/smbd/trans2.c >@@ -154,6 +154,16 @@ uint64_t get_FileIndex(connection_struct *conn, const SMB_STRUCT_STAT *psbuf) > 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[] = { >@@ -161,6 +171,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.12.3 > > >From a8183661cc40f3e91bbfa3aa18d8253e3acf6058 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Thu, 13 Jan 2022 17:03:02 +0100 >Subject: [PATCH 4/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> >--- > source3/modules/vfs_fruit.c | 117 ++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 102 insertions(+), 15 deletions(-) > >diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c >index 4148e86b5df..e7a30d7604b 100644 >--- a/source3/modules/vfs_fruit.c >+++ b/source3/modules/vfs_fruit.c >@@ -413,6 +413,95 @@ static struct adouble *ad_init(TALLOC_CTX *ctx, vfs_handle_struct *handle, > static int ad_write(struct adouble *ad, const char *path); > static int adouble_path(TALLOC_CTX *ctx, const char *path_in, char **path_out); > >+/* >+ * 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 > * >@@ -420,8 +509,15 @@ static int adouble_path(TALLOC_CTX *ctx, const char *path_in, char **path_out); > **/ > 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; >@@ -485,7 +581,6 @@ static int ad_setdate(struct adouble *ad, unsigned int dateoff, uint32_t date) > return 0; > } > >- > /** > * Map on-disk AppleDouble id to enumerated id > **/ >@@ -629,6 +724,7 @@ static bool ad_unpack(struct adouble *ad, const int nentries, size_t filesize) > > /* now, read in the entry bits */ > for (i = 0; i < adentries; i++) { >+ bool ok; > eid = RIVAL(ad->ad_data, AD_HEADER_LEN + (i * AD_ENTRY_LEN)); > eid = get_eid(eid); > off = RIVAL(ad->ad_data, AD_HEADER_LEN + (i * AD_ENTRY_LEN) + 4); >@@ -650,20 +746,11 @@ static bool ad_unpack(struct adouble *ad, const int nentries, size_t filesize) > 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.12.3 > > >From 660d6a200961e39bfcfe093c1e37a9b622a7636a Mon Sep 17 00:00:00 2001 >From: Noel Power <noel.power@suse.com> >Date: Fri, 21 Jan 2022 18:52:43 +0000 >Subject: [PATCH 5/5] squash: Adjust previous patch to cater for non handling > of emedded xattrs > >--- > source3/modules/vfs_fruit.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > >diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c >index e7a30d7604b..c65520c1ea9 100644 >--- a/source3/modules/vfs_fruit.c >+++ b/source3/modules/vfs_fruit.c >@@ -495,6 +495,16 @@ static bool ad_entry_check_size(uint32_t eid, > /* wrap around */ > return false; > } >+ /* >+ * This version of samba doesn't >+ * support marshalled xattrs for FinderInfo >+ * but the buffer *must* support the first 32 bytes >+ * any extra length of this entry will be discarded (along with >+ * the embedded xattrs) >+ */ >+ if (eid == ADEID_FINDERI) { >+ got_len = ADEDLEN_FINDERI; >+ } > if (off + got_len > bufsize) { > /* overflow */ > return false; >-- >2.12.3 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=17117">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
npower
:
review?
(
slow
)
Actions:
View
Attachments on
bug 14914
:
17008
|
17084
|
17087
|
17088
|
17089
|
17090
|
17091
|
17092
|
17093
|
17094
|
17115
|
17116
| 17117 |
17119
|
17128