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>

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 6ebe325618e9e8a06393bd66c3d65301bbb64082..d04c37a8da06da53510b4e8ee99b5b07efdfc6d0 100644 (file)
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -341,7 +341,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;
@@ -411,6 +410,23 @@ 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);
 
+/**
+ * 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
  **/
@@ -419,18 +435,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);
@@ -444,9 +461,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;
@@ -458,7 +477,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;
 }
@@ -944,6 +963,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;
@@ -973,9 +995,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) {
@@ -1582,7 +1613,7 @@ static bool ad_empty_finderinfo(const struct adouble *ad)
        char emptybuf[ADEDLEN_FINDERI] = {0};
        char *fi = NULL;
 
-       fi = ad_entry(ad, ADEID_FINDERI);
+       fi = ad_get_entry(ad, ADEID_FINDERI);
        if (fi == NULL) {
                DBG_ERR("Missing FinderInfo in struct adouble [%p]\n", ad);
                return false;
@@ -2007,23 +2038,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(
@@ -2034,6 +2074,7 @@ static NTSTATUS readdir_attr_macmeta(struct vfs_handle_struct *handle,
                }
        }
 
+out:
        TALLOC_FREE(ad);
        return status;
 }
@@ -2768,6 +2809,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;
 
                ai = afpinfo_new(talloc_tos());
                if (ai == NULL) {
@@ -2781,9 +2823,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;
@@ -2922,9 +2971,17 @@ static ssize_t fruit_pwrite(vfs_handle_struct *handle,
        }
 
        if (ad->ad_type == ADOUBLE_META) {
-               memcpy(ad_entry(ad, ADEID_FINDERI),
-                      &ai->afpi_FinderInfo[0], ADEDLEN_FINDERI);
+               char *p = NULL;
 
+               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);
                rc = ad_write(ad, name);
        } else {
                len = SMB_VFS_NEXT_PWRITE(handle, fsp, data, n,
@@ -3253,14 +3310,16 @@ static NTSTATUS fruit_streaminfo(vfs_handle_struct *handle,
        DEBUG(10, ("fruit_streaminfo called for %s\n", smb_fname->base_name));
 
        if (config->meta == FRUIT_META_NETATALK) {
+               bool ok;
+
                ad = ad_get(talloc_tos(), handle,
                            smb_fname->base_name, ADOUBLE_META);
-               if (ad && !ad_empty_finderinfo(ad)) {
-                       if (!add_fruit_stream(
-                                   mem_ctx, pnum_streams, pstreams,
-                                   AFPINFO_STREAM_NAME, AFP_INFO_SIZE,
-                                   smb_roundup(handle->conn,
-                                               AFP_INFO_SIZE))) {
+               if ((ad != NULL) && !ad_empty_finderinfo(ad)) {
+                       ok = add_fruit_stream(
+                               mem_ctx, pnum_streams, pstreams,
+                               AFPINFO_STREAM_NAME, AFP_INFO_SIZE,
+                               smb_roundup(handle->conn, AFP_INFO_SIZE));
+                       if (!ok) {
                                TALLOC_FREE(ad);
                                return NT_STATUS_NO_MEMORY;
                        }