| From 687106e4269c2e926e0452a88c2d72ac95779a95 Mon Sep 17 00:00:00 2001 |
| From: Andrew Walker <awalker@ixsystems.com> |
| Date: Mon, 11 Apr 2022 09:13:09 -0400 |
| Subject: [PATCH 1/4] add handling for cases where ad_entry() returns NULL |
| |
| With recent CVE fixes, ad_enty() may now return NULL. This |
| commit adds basic error handling for these cases and asserting |
| where such a return is totally unexpected. In case of |
| ad_getid() and ad_forcegetid(), return CNID_INVALID rather |
| than 0 to clarify for future people investigating this that |
| a 0 here is an indication of error. |
| |
| In case of new_ad_header(), the valid_data_len of the |
| adouble data may still be zero. This causes subsequent |
| ad_entry() calls within new_ad_header() to fail. As such, |
| overwrite valid_data-Len with AD_DATASZ2 or AD_DATASZ_EA |
| depending on how adouble data is stored on disk. |
| |
| Another side-effect of the fix is that ad_entry() for |
| ADEID_DID on ea-backed adouble data will return NULL. In |
| this case, add explicit check for the backend before |
| attempting to get the DID. |
| |
| Signed-off-by: Andrew Walker <awalker@ixsystems.com> |
| --- |
| etc/afpd/directory.c | 15 +++- |
| etc/afpd/file.c | 25 +++++-- |
| etc/afpd/volume.c | 7 +- |
| etc/cnid_dbd/cmd_dbd_scanvol.c | 9 ++- |
| libatalk/adouble/ad_attr.c | 130 +++++++++++++++++++++++++++------ |
| libatalk/adouble/ad_conv.c | 4 +- |
| libatalk/adouble/ad_date.c | 17 ++++- |
| libatalk/adouble/ad_flush.c | 27 ++++++- |
| libatalk/adouble/ad_open.c | 39 ++++++---- |
| 9 files changed, 215 insertions(+), 58 deletions(-) |
| |
| --- a/etc/afpd/directory.c |
| +++ b/etc/afpd/directory.c |
| @@ -1426,6 +1426,7 @@ int getdirparams(const AFPObj *obj, |
| struct maccess ma; |
| struct adouble ad; |
| char *data, *l_nameoff = NULL, *utf_nameoff = NULL; |
| + char *ade = NULL; |
| int bit = 0, isad = 0; |
| uint32_t aint; |
| uint16_t ashort; |
| @@ -1520,7 +1521,10 @@ int getdirparams(const AFPObj *obj, |
| |
| case DIRPBIT_FINFO : |
| if ( isad ) { |
| - memcpy( data, ad_entry( &ad, ADEID_FINDERI ), 32 ); |
| + ade = ad_entry(&ad, ADEID_FINDERI); |
| + AFP_ASSERT(ade != NULL); |
| + |
| + memcpy( data, ade, 32 ); |
| } else { /* no appledouble */ |
| memset( data, 0, 32 ); |
| /* dot files are by default visible */ |
| @@ -1744,6 +1748,7 @@ int setdirparams(struct vol *vol, struct |
| struct timeval tv; |
| |
| char *upath; |
| + char *ade = NULL; |
| struct dir *dir; |
| int bit, isad = 0; |
| int cdate, bdate; |
| @@ -1905,6 +1910,8 @@ int setdirparams(struct vol *vol, struct |
| fflags &= htons(~FINDERINFO_ISHARED); |
| memcpy(finder_buf + FINDERINFO_FRFLAGOFF, &fflags, sizeof(uint16_t)); |
| /* #2802236 end */ |
| + ade = ad_entry(&ad, ADEID_FINDERI); |
| + AFP_ASSERT(ade != NULL); |
| |
| if ( dir->d_did == DIRDID_ROOT ) { |
| /* |
| @@ -1915,10 +1922,10 @@ int setdirparams(struct vol *vol, struct |
| * behavior one sees when mounting above another mount |
| * point. |
| */ |
| - memcpy( ad_entry( &ad, ADEID_FINDERI ), finder_buf, 10 ); |
| - memcpy( ad_entry( &ad, ADEID_FINDERI ) + 14, finder_buf + 14, 18 ); |
| + memcpy( ade, finder_buf, 10 ); |
| + memcpy( ade + 14, finder_buf + 14, 18 ); |
| } else { |
| - memcpy( ad_entry( &ad, ADEID_FINDERI ), finder_buf, 32 ); |
| + memcpy( ade, finder_buf, 32 ); |
| } |
| } |
| break; |
| --- a/etc/afpd/file.c |
| +++ b/etc/afpd/file.c |
| @@ -296,6 +296,7 @@ int getmetadata(const AFPObj *obj, |
| { |
| char *data, *l_nameoff = NULL, *upath; |
| char *utf_nameoff = NULL; |
| + char *ade = NULL; |
| int bit = 0; |
| uint32_t aint; |
| cnid_t id = 0; |
| @@ -497,7 +498,10 @@ int getmetadata(const AFPObj *obj, |
| } |
| else { |
| if ( adp ) { |
| - memcpy(fdType, ad_entry( adp, ADEID_FINDERI ), 4 ); |
| + ade = ad_entry(adp, ADEID_FINDERI); |
| + AFP_ASSERT(ade != NULL); |
| + |
| + memcpy(fdType, ade, 4); |
| |
| if ( memcmp( fdType, "TEXT", 4 ) == 0 ) { |
| achar = '\x04'; |
| @@ -576,8 +580,19 @@ int getmetadata(const AFPObj *obj, |
| 10.3 clients freak out. */ |
| |
| aint = st->st_mode; |
| - if (adp) { |
| - memcpy(fdType, ad_entry( adp, ADEID_FINDERI ), 4 ); |
| + /* |
| + * ad_open() does not initialize adouble header |
| + * for symlinks. Hence this should be skipped to |
| + * avoid AFP_ASSERT here. Decision was made to |
| + * not alter ad_open() behavior so that |
| + * improper ops on symlink adoubles will be |
| + * more visible (assert). |
| + */ |
| + if (adp && (ad_meta_fileno(adp) != AD_SYMLINK)) { |
| + ade = ad_entry(adp, ADEID_FINDERI); |
| + AFP_ASSERT(ade != NULL); |
| + |
| + memcpy(fdType, ade, 4); |
| if ( memcmp( fdType, "slnk", 4 ) == 0 ) { |
| aint |= S_IFLNK; |
| } |
| @@ -839,6 +854,7 @@ int setfilparams(const AFPObj *obj, stru |
| struct extmap *em; |
| int bit, isad = 1, err = AFP_OK; |
| char *upath; |
| + char *ade = NULL; |
| u_char achar, *fdType, xyy[4]; /* uninitialized, OK 310105 */ |
| uint16_t ashort, bshort, oshort; |
| uint32_t aint; |
| @@ -989,7 +1005,7 @@ int setfilparams(const AFPObj *obj, stru |
| /* second try with adouble open |
| */ |
| if (ad_open(adp, upath, ADFLAGS_HF | ADFLAGS_RDWR | ADFLAGS_CREATE, 0666) < 0) { |
| - LOG(log_debug, logtype_afpd, "setfilparams: ad_open_metadata error"); |
| + LOG(log_debug, logtype_afpd, "setfilparams: ad_open_metadata error: %s", strerror(errno)); |
| /* |
| * For some things, we don't need an adouble header: |
| * - change of modification date |
| @@ -1021,6 +1037,9 @@ int setfilparams(const AFPObj *obj, stru |
| |
| switch( bit ) { |
| case FILPBIT_ATTR : |
| + if (isad == 0) { |
| + break; |
| + } |
| ad_getattr(adp, &bshort); |
| oshort = bshort; |
| if ( ntohs( ashort ) & ATTRBIT_SETCLR ) { |
| @@ -1034,15 +1053,26 @@ int setfilparams(const AFPObj *obj, stru |
| ad_setattr(adp, bshort); |
| break; |
| case FILPBIT_CDATE : |
| + if (isad == 0) { |
| + break; |
| + } |
| ad_setdate(adp, AD_DATE_CREATE, cdate); |
| break; |
| case FILPBIT_MDATE : |
| break; |
| case FILPBIT_BDATE : |
| + if (isad == 0) { |
| + break; |
| + } |
| ad_setdate(adp, AD_DATE_BACKUP, bdate); |
| break; |
| case FILPBIT_FINFO : |
| - if (default_type( ad_entry( adp, ADEID_FINDERI )) |
| + if (isad == 0) { |
| + break; |
| + } |
| + ade = ad_entry(adp, ADEID_FINDERI); |
| + AFP_ASSERT(ade != NULL); |
| + if (default_type(ade) |
| && ( |
| ((em = getextmap( path->m_name )) && |
| !memcmp(finder_buf, em->em_type, sizeof( em->em_type )) && |
| @@ -1053,7 +1083,7 @@ int setfilparams(const AFPObj *obj, stru |
| )) { |
| memcpy(finder_buf, ufinderi, 8 ); |
| } |
| - memcpy(ad_entry( adp, ADEID_FINDERI ), finder_buf, 32 ); |
| + memcpy(ade, finder_buf, 32 ); |
| break; |
| case FILPBIT_UNIXPR : |
| if (upriv_bit) { |
| @@ -1061,9 +1091,15 @@ int setfilparams(const AFPObj *obj, stru |
| } |
| break; |
| case FILPBIT_PDINFO : |
| + if (isad == 0) { |
| + break; |
| + } |
| + ade = ad_entry(adp, ADEID_FINDERI); |
| + AFP_ASSERT(ade != NULL); |
| + |
| if (obj->afp_version < 30) { /* else it's UTF8 name */ |
| - memcpy(ad_entry( adp, ADEID_FINDERI ), fdType, 4 ); |
| - memcpy(ad_entry( adp, ADEID_FINDERI ) + 4, "pdos", 4 ); |
| + memcpy(ade, fdType, 4 ); |
| + memcpy(ade + 4, "pdos", 4 ); |
| break; |
| } |
| /* fallthrough */ |
| --- a/etc/afpd/volume.c |
| +++ b/etc/afpd/volume.c |
| @@ -305,6 +305,7 @@ static int getvolparams(const AFPObj *ob |
| VolSpace xbfree, xbtotal; /* extended bytes */ |
| char *data, *nameoff = NULL; |
| char *slash; |
| + char *ade = NULL; |
| |
| LOG(log_debug, logtype_afpd, "getvolparams: Volume '%s'", vol->v_localname); |
| |
| @@ -328,8 +329,10 @@ static int getvolparams(const AFPObj *ob |
| slash = vol->v_path; |
| if (ad_getentryoff(&ad, ADEID_NAME)) { |
| ad_setentrylen( &ad, ADEID_NAME, strlen( slash )); |
| - memcpy(ad_entry( &ad, ADEID_NAME ), slash, |
| - ad_getentrylen( &ad, ADEID_NAME )); |
| + ade = ad_entry(&ad, ADEID_NAME); |
| + AFP_ASSERT(ade != NULL); |
| + |
| + memcpy(ade, slash, ad_getentrylen( &ad, ADEID_NAME )); |
| } |
| vol_setdate(vol->v_vid, &ad, st->st_mtime); |
| ad_flush(&ad); |
| --- a/etc/cnid_dbd/cmd_dbd_scanvol.c |
| +++ b/etc/cnid_dbd/cmd_dbd_scanvol.c |
| @@ -560,6 +560,7 @@ static int read_addir(void) |
| static cnid_t check_cnid(const char *name, cnid_t did, struct stat *st, int adfile_ok) |
| { |
| int adflags = ADFLAGS_HF; |
| + int err; |
| cnid_t db_cnid, ad_cnid; |
| struct adouble ad; |
| |
| @@ -602,7 +603,13 @@ static cnid_t check_cnid(const char *nam |
| cwdbuf, name, strerror(errno)); |
| return CNID_INVALID; |
| } |
| - ad_setid( &ad, st->st_dev, st->st_ino, db_cnid, did, stamp); |
| + err = ad_setid( &ad, st->st_dev, st->st_ino, db_cnid, did, stamp); |
| + if (err == -1) { |
| + dbd_log(LOGSTD, "Error setting new CNID, malformed adouble: '%s/%s'", |
| + cwdbuf, name); |
| + ad_close(&ad, ADFLAGS_HF); |
| + return CNID_INVALID; |
| + } |
| ad_flush(&ad); |
| ad_close(&ad, ADFLAGS_HF); |
| } |
| --- a/libatalk/adouble/ad_attr.c |
| +++ b/libatalk/adouble/ad_attr.c |
| @@ -2,8 +2,10 @@ |
| #include "config.h" |
| #endif /* HAVE_CONFIG_H */ |
| |
| +#include <stdlib.h> |
| #include <string.h> |
| #include <arpa/inet.h> |
| +#include <atalk/util.h> |
| #include <atalk/adouble.h> |
| #include <atalk/logger.h> |
| |
| @@ -22,10 +24,17 @@ int ad_getattr(const struct adouble *ad, |
| *attr = 0; |
| |
| if (ad_getentryoff(ad, ADEID_AFPFILEI)) { |
| - memcpy(attr, ad_entry(ad, ADEID_AFPFILEI) + AFPFILEIOFF_ATTR, 2); |
| + char *adp = NULL; |
| + |
| + adp = ad_entry(ad, ADEID_AFPFILEI); |
| + AFP_ASSERT(adp != NULL); |
| + memcpy(attr, adp + AFPFILEIOFF_ATTR, 2); |
| |
| /* Now get opaque flags from FinderInfo */ |
| - memcpy(&fflags, ad_entry(ad, ADEID_FINDERI) + FINDERINFO_FRFLAGOFF, 2); |
| + adp = ad_entry(ad, ADEID_FINDERI); |
| + AFP_ASSERT(adp != NULL); |
| + memcpy(&fflags, adp + FINDERINFO_FRFLAGOFF, 2); |
| + |
| if (fflags & htons(FINDERINFO_INVISIBLE)) |
| *attr |= htons(ATTRBIT_INVISIBLE); |
| else |
| @@ -61,10 +70,15 @@ int ad_setattr(const struct adouble *ad, |
| attr &= ~(ATTRBIT_MULTIUSER | ATTRBIT_NOWRITE | ATTRBIT_NOCOPY); |
| |
| if (ad_getentryoff(ad, ADEID_AFPFILEI) && ad_getentryoff(ad, ADEID_FINDERI)) { |
| - memcpy(ad_entry(ad, ADEID_AFPFILEI) + AFPFILEIOFF_ATTR, &attr, sizeof(attr)); |
| + char *adp = NULL; |
| + |
| + adp = ad_entry(ad, ADEID_FINDERI); |
| + AFP_ASSERT(adp != NULL); |
| + |
| + memcpy(adp + AFPFILEIOFF_ATTR, &attr, sizeof(attr)); |
| |
| /* Now set opaque flags in FinderInfo too */ |
| - memcpy(&fflags, ad_entry(ad, ADEID_FINDERI) + FINDERINFO_FRFLAGOFF, 2); |
| + memcpy(&fflags, adp + FINDERINFO_FRFLAGOFF, 2); |
| if (attr & htons(ATTRBIT_INVISIBLE)) |
| fflags |= htons(FINDERINFO_INVISIBLE); |
| else |
| @@ -77,7 +91,7 @@ int ad_setattr(const struct adouble *ad, |
| } else |
| fflags &= htons(~FINDERINFO_ISHARED); |
| |
| - memcpy(ad_entry(ad, ADEID_FINDERI) + FINDERINFO_FRFLAGOFF, &fflags, 2); |
| + memcpy(adp + FINDERINFO_FRFLAGOFF, &fflags, 2); |
| } |
| |
| return 0; |
| @@ -86,54 +100,114 @@ int ad_setattr(const struct adouble *ad, |
| /* -------------- |
| * save file/folder ID in AppleDoubleV2 netatalk private parameters |
| * return 1 if resource fork has been modified |
| + * return -1 on error. |
| */ |
| int ad_setid (struct adouble *adp, const dev_t dev, const ino_t ino , const uint32_t id, const cnid_t did, const void *stamp) |
| { |
| uint32_t tmp; |
| + char *ade = NULL; |
| |
| ad_setentrylen( adp, ADEID_PRIVID, sizeof(id)); |
| tmp = id; |
| if (adp->ad_vers == AD_VERSION_EA) |
| tmp = htonl(tmp); |
| - memcpy(ad_entry( adp, ADEID_PRIVID ), &tmp, sizeof(tmp)); |
| + |
| + ade = ad_entry(adp, ADEID_PRIVID); |
| + if (ade == NULL) { |
| + LOG(log_warning, logtype_ad, "ad_setid: failed to set ADEID_PRIVID\n"); |
| + return -1; |
| + } |
| + memcpy(ade, &tmp, sizeof(tmp)); |
| |
| ad_setentrylen( adp, ADEID_PRIVDEV, sizeof(dev_t)); |
| + ade = ad_entry(adp, ADEID_PRIVDEV); |
| + if (ade == NULL) { |
| + LOG(log_warning, logtype_ad, "ad_setid: failed to set ADEID_PRIVDEV\n"); |
| + return -1; |
| + } |
| + |
| if ((adp->ad_options & ADVOL_NODEV)) { |
| - memset(ad_entry( adp, ADEID_PRIVDEV ), 0, sizeof(dev_t)); |
| + memset(ade, 0, sizeof(dev_t)); |
| } else { |
| - memcpy(ad_entry( adp, ADEID_PRIVDEV ), &dev, sizeof(dev_t)); |
| + memcpy(ade, &dev, sizeof(dev_t)); |
| } |
| |
| ad_setentrylen( adp, ADEID_PRIVINO, sizeof(ino_t)); |
| - memcpy(ad_entry( adp, ADEID_PRIVINO ), &ino, sizeof(ino_t)); |
| |
| - ad_setentrylen( adp, ADEID_DID, sizeof(did)); |
| - memcpy(ad_entry( adp, ADEID_DID ), &did, sizeof(did)); |
| + ade = ad_entry(adp, ADEID_PRIVINO); |
| + if (ade == NULL) { |
| + LOG(log_warning, logtype_ad, "ad_setid: failed to set ADEID_PRIVINO\n"); |
| + return -1; |
| + } |
| + memcpy(ade, &ino, sizeof(ino_t)); |
| + |
| + if (adp->ad_vers != AD_VERSION_EA) { |
| + ad_setentrylen( adp, ADEID_DID, sizeof(did)); |
| + |
| + ade = ad_entry(adp, ADEID_DID); |
| + if (ade == NULL) { |
| + LOG(log_warning, logtype_ad, "ad_setid: failed to set ADEID_DID\n"); |
| + return -1; |
| + } |
| + memcpy(ade, &did, sizeof(did)); |
| + } |
| |
| ad_setentrylen( adp, ADEID_PRIVSYN, ADEDLEN_PRIVSYN); |
| - memcpy(ad_entry( adp, ADEID_PRIVSYN ), stamp, ADEDLEN_PRIVSYN); |
| + ade = ad_entry(adp, ADEID_PRIVSYN); |
| + if (ade == NULL) { |
| + LOG(log_warning, logtype_ad, "ad_setid: failed to set ADEID_PRIVSYN\n"); |
| + return -1; |
| + } |
| + memcpy(ade, stamp, ADEDLEN_PRIVSYN); |
| |
| return 1; |
| } |
| |
| -/* ----------------------------- */ |
| +/* |
| + * Retrieve stored file / folder. Callers should treat a return of CNID_INVALID (0) as an invalid value. |
| + */ |
| uint32_t ad_getid (struct adouble *adp, const dev_t st_dev, const ino_t st_ino , const cnid_t did, const void *stamp _U_) |
| { |
| uint32_t aint = 0; |
| dev_t dev; |
| ino_t ino; |
| - cnid_t a_did; |
| + cnid_t a_did = 0; |
| |
| if (adp) { |
| if (sizeof(dev_t) == ad_getentrylen(adp, ADEID_PRIVDEV)) { |
| - memcpy(&dev, ad_entry(adp, ADEID_PRIVDEV), sizeof(dev_t)); |
| - memcpy(&ino, ad_entry(adp, ADEID_PRIVINO), sizeof(ino_t)); |
| - memcpy(&a_did, ad_entry(adp, ADEID_DID), sizeof(cnid_t)); |
| + char *ade = NULL; |
| + ade = ad_entry(adp, ADEID_PRIVDEV); |
| + if (ade == NULL) { |
| + LOG(log_warning, logtype_ad, "ad_getid: failed to retrieve ADEID_PRIVDEV\n"); |
| + return CNID_INVALID; |
| + } |
| + memcpy(&dev, ade, sizeof(dev_t)); |
| + ade = ad_entry(adp, ADEID_PRIVINO); |
| + if (ade == NULL) { |
| + LOG(log_warning, logtype_ad, "ad_getid: failed to retrieve ADEID_PRIVINO\n"); |
| + return CNID_INVALID; |
| + } |
| + memcpy(&ino, ade, sizeof(ino_t)); |
| + |
| + if (adp->ad_vers != AD_VERSION_EA) { |
| + /* ADEID_DID is not stored for AD_VERSION_EA */ |
| + ade = ad_entry(adp, ADEID_DID); |
| + if (ade == NULL) { |
| + LOG(log_warning, logtype_ad, "ad_getid: failed to retrieve ADEID_DID\n"); |
| + return CNID_INVALID; |
| + } |
| + memcpy(&a_did, ade, sizeof(cnid_t)); |
| + } |
| |
| if (((adp->ad_options & ADVOL_NODEV) || (dev == st_dev)) |
| && ino == st_ino |
| - && (!did || a_did == did) ) { |
| - memcpy(&aint, ad_entry(adp, ADEID_PRIVID), sizeof(aint)); |
| + && (!did || a_did == 0 || a_did == did) ) { |
| + ade = ad_entry(adp, ADEID_PRIVID); |
| + if (ade == NULL) { |
| + LOG(log_warning, logtype_ad, "ad_getid: failed to retrieve ADEID_PRIVID\n"); |
| + return CNID_INVALID; |
| + } |
| + memcpy(&aint, ade, sizeof(aint)); |
| if (adp->ad_vers == AD_VERSION2) |
| return aint; |
| else |
| @@ -141,7 +215,7 @@ uint32_t ad_getid (struct adouble *adp, |
| } |
| } |
| } |
| - return 0; |
| + return CNID_INVALID; |
| } |
| |
| /* ----------------------------- */ |
| @@ -150,13 +224,18 @@ uint32_t ad_forcegetid (struct adouble * |
| uint32_t aint = 0; |
| |
| if (adp) { |
| - memcpy(&aint, ad_entry(adp, ADEID_PRIVID), sizeof(aint)); |
| + char *ade = NULL; |
| + ade = ad_entry(adp, ADEID_PRIVID); |
| + if (ade == NULL) { |
| + return CNID_INVALID; |
| + } |
| + memcpy(&aint, ade, sizeof(aint)); |
| if (adp->ad_vers == AD_VERSION2) |
| return aint; |
| else |
| return ntohl(aint); |
| } |
| - return 0; |
| + return CNID_INVALID; |
| } |
| |
| /* ----------------- |
| @@ -168,8 +247,13 @@ int ad_setname(struct adouble *ad, const |
| if ((len = strlen(path)) > ADEDLEN_NAME) |
| len = ADEDLEN_NAME; |
| if (path && ad_getentryoff(ad, ADEID_NAME)) { |
| + char *ade = NULL; |
| ad_setentrylen( ad, ADEID_NAME, len); |
| - memcpy(ad_entry( ad, ADEID_NAME ), path, len); |
| + ade = ad_entry(ad, ADEID_NAME); |
| + if (ade == NULL) { |
| + return -1; |
| + } |
| + memcpy(ade, path, len); |
| return 1; |
| } |
| return 0; |
| --- a/libatalk/adouble/ad_conv.c |
| +++ b/libatalk/adouble/ad_conv.c |
| @@ -93,6 +93,7 @@ static int ad_conv_v22ea_hf(const char * |
| goto copy; |
| if (ad_getentryoff(&adv2, ADEID_FINDERI) |
| && (ad_getentrylen(&adv2, ADEID_FINDERI) == ADEDLEN_FINDERI) |
| + && (ad_entry(&adv2, ADEID_FINDERI) != NULL) |
| && (memcmp(ad_entry(&adv2, ADEID_FINDERI), emptyad, ADEDLEN_FINDERI) != 0)) |
| goto copy; |
| if (ad_getentryoff(&adv2, ADEID_FILEDATESI)) { |
| @@ -101,7 +102,7 @@ static int ad_conv_v22ea_hf(const char * |
| if ((ctime != mtime) || (mtime != sp->st_mtime)) |
| goto copy; |
| } |
| - if (ad_getentryoff(&adv2, ADEID_AFPFILEI)) { |
| + if (ad_getentryoff(&adv2, ADEID_AFPFILEI) && (ad_entry(&adv2, ADEID_AFPFILEI) != NULL)) { |
| if (memcmp(ad_entry(&adv2, ADEID_AFPFILEI), &afpinfo, ADEDLEN_AFPFILEI) != 0) |
| goto copy; |
| } |
| @@ -115,6 +116,7 @@ copy: |
| EC_ZERO_LOGSTR( ad_open(&adea, path, adflags | ADFLAGS_HF | ADFLAGS_RDWR | ADFLAGS_CREATE), |
| "ad_conv_v22ea_hf(\"%s\"): error creating metadata EA: %s", |
| fullpathname(path), strerror(errno)); |
| + AFP_ASSERT(ad_refresh(path, &adea) == 0); |
| EC_ZERO_LOG( ad_copy_header(&adea, &adv2) ); |
| ad_flush(&adea); |
| |
| --- a/libatalk/adouble/ad_date.c |
| +++ b/libatalk/adouble/ad_date.c |
| @@ -10,6 +10,7 @@ int ad_setdate(struct adouble *ad, |
| unsigned int dateoff, uint32_t date) |
| { |
| int xlate = (dateoff & AD_DATE_UNIX); |
| + char *ade = NULL; |
| |
| dateoff &= AD_DATE_MASK; |
| if (xlate) |
| @@ -20,7 +21,12 @@ int ad_setdate(struct adouble *ad, |
| |
| if (dateoff > AD_DATE_ACCESS) |
| return -1; |
| - memcpy(ad_entry(ad, ADEID_FILEDATESI) + dateoff, &date, sizeof(date)); |
| + |
| + ade = ad_entry(ad, ADEID_FILEDATESI); |
| + if (ade == NULL) { |
| + return -1; |
| + } |
| + memcpy(ade + dateoff, &date, sizeof(date)); |
| |
| return 0; |
| } |
| @@ -29,6 +35,7 @@ int ad_getdate(const struct adouble *ad, |
| unsigned int dateoff, uint32_t *date) |
| { |
| int xlate = (dateoff & AD_DATE_UNIX); |
| + char *ade = NULL; |
| |
| dateoff &= AD_DATE_MASK; |
| if (!ad_getentryoff(ad, ADEID_FILEDATESI)) |
| @@ -36,7 +43,13 @@ int ad_getdate(const struct adouble *ad, |
| |
| if (dateoff > AD_DATE_ACCESS) |
| return -1; |
| - memcpy(date, ad_entry(ad, ADEID_FILEDATESI) + dateoff, sizeof(uint32_t)); |
| + |
| + |
| + ade = ad_entry(ad, ADEID_FILEDATESI); |
| + if (ade == NULL) { |
| + return -1; |
| + } |
| + memcpy(date, ade + dateoff, sizeof(uint32_t)); |
| |
| if (xlate) |
| *date = AD_DATE_TO_UNIX(*date); |
| --- a/libatalk/adouble/ad_flush.c |
| +++ b/libatalk/adouble/ad_flush.c |
| @@ -151,6 +151,7 @@ int ad_rebuild_adouble_header_osx(struct |
| uint32_t temp; |
| uint16_t nent; |
| char *buf; |
| + char *ade = NULL; |
| |
| LOG(log_debug, logtype_ad, "ad_rebuild_adouble_header_osx"); |
| |
| @@ -184,7 +185,10 @@ int ad_rebuild_adouble_header_osx(struct |
| memcpy(buf, &temp, sizeof( temp )); |
| buf += sizeof( temp ); |
| |
| - memcpy(adbuf + ADEDOFF_FINDERI_OSX, ad_entry(ad, ADEID_FINDERI), ADEDLEN_FINDERI); |
| + ade = ad_entry(ad, ADEID_FINDERI); |
| + AFP_ASSERT(ade != NULL); |
| + |
| + memcpy(adbuf + ADEDOFF_FINDERI_OSX, ade, ADEDLEN_FINDERI); |
| |
| /* rfork */ |
| temp = htonl( EID_DISK(ADEID_RFORK) ); |
| @@ -211,8 +215,12 @@ int ad_copy_header(struct adouble *add, |
| { |
| uint32_t eid; |
| uint32_t len; |
| + char *src = NULL; |
| + char *dst = NULL; |
| |
| for ( eid = 0; eid < ADEID_MAX; eid++ ) { |
| + src = dst = NULL; |
| + |
| if ( ads->ad_eid[ eid ].ade_off == 0 || add->ad_eid[ eid ].ade_off == 0 ) |
| continue; |
| |
| @@ -226,17 +234,28 @@ int ad_copy_header(struct adouble *add, |
| continue; |
| default: |
| ad_setentrylen( add, eid, len ); |
| - memcpy( ad_entry( add, eid ), ad_entry( ads, eid ), len ); |
| + dst = ad_entry(add, eid); |
| + AFP_ASSERT(dst != NULL); |
| + |
| + src = ad_entry(ads, eid); |
| + AFP_ASSERT(src != NULL); |
| + |
| + memcpy( dst, src, len ); |
| } |
| } |
| add->ad_rlen = ads->ad_rlen; |
| |
| if (((ads->ad_vers == AD_VERSION2) && (add->ad_vers == AD_VERSION_EA)) |
| || ((ads->ad_vers == AD_VERSION_EA) && (add->ad_vers == AD_VERSION2))) { |
| + src = dst = NULL; |
| cnid_t id; |
| - memcpy(&id, ad_entry(add, ADEID_PRIVID), sizeof(cnid_t)); |
| + |
| + dst = ad_entry(add, ADEID_PRIVID); |
| + AFP_ASSERT(dst != NULL); |
| + |
| + memcpy(&id, dst, sizeof(cnid_t)); |
| id = htonl(id); |
| - memcpy(ad_entry(add, ADEID_PRIVID), &id, sizeof(cnid_t)); |
| + memcpy(dst, &id, sizeof(cnid_t)); |
| } |
| return 0; |
| } |
| --- a/libatalk/adouble/ad_open.c |
| +++ b/libatalk/adouble/ad_open.c |
| @@ -140,17 +140,17 @@ static struct adouble_fops ad_adouble_ea |
| |
| static const struct entry entry_order2[ADEID_NUM_V2 + 1] = { |
| {ADEID_NAME, ADEDOFF_NAME_V2, ADEDLEN_INIT}, |
| - {ADEID_COMMENT, ADEDOFF_COMMENT_V2, ADEDLEN_INIT}, |
| + {ADEID_COMMENT, ADEDOFF_COMMENT_V2, ADEDLEN_COMMENT}, |
| {ADEID_FILEDATESI, ADEDOFF_FILEDATESI, ADEDLEN_FILEDATESI}, |
| {ADEID_FINDERI, ADEDOFF_FINDERI_V2, ADEDLEN_FINDERI}, |
| {ADEID_DID, ADEDOFF_DID, ADEDLEN_DID}, |
| {ADEID_AFPFILEI, ADEDOFF_AFPFILEI, ADEDLEN_AFPFILEI}, |
| {ADEID_SHORTNAME, ADEDOFF_SHORTNAME, ADEDLEN_INIT}, |
| {ADEID_PRODOSFILEI, ADEDOFF_PRODOSFILEI, ADEDLEN_PRODOSFILEI}, |
| - {ADEID_PRIVDEV, ADEDOFF_PRIVDEV, ADEDLEN_INIT}, |
| - {ADEID_PRIVINO, ADEDOFF_PRIVINO, ADEDLEN_INIT}, |
| - {ADEID_PRIVSYN, ADEDOFF_PRIVSYN, ADEDLEN_INIT}, |
| - {ADEID_PRIVID, ADEDOFF_PRIVID, ADEDLEN_INIT}, |
| + {ADEID_PRIVDEV, ADEDOFF_PRIVDEV, ADEDLEN_PRIVDEV}, |
| + {ADEID_PRIVINO, ADEDOFF_PRIVINO, ADEDLEN_PRIVINO}, |
| + {ADEID_PRIVSYN, ADEDOFF_PRIVSYN, ADEDLEN_PRIVSYN}, |
| + {ADEID_PRIVID, ADEDOFF_PRIVID, ADEDLEN_PRIVID}, |
| {ADEID_RFORK, ADEDOFF_RFORK_V2, ADEDLEN_INIT}, |
| {0, 0, 0} |
| }; |
| @@ -158,13 +158,13 @@ static const struct entry entry_order2[A |
| /* Using Extended Attributes */ |
| static const struct entry entry_order_ea[ADEID_NUM_EA + 1] = { |
| {ADEID_FINDERI, ADEDOFF_FINDERI_EA, ADEDLEN_FINDERI}, |
| - {ADEID_COMMENT, ADEDOFF_COMMENT_EA, ADEDLEN_INIT}, |
| + {ADEID_COMMENT, ADEDOFF_COMMENT_EA, ADEDLEN_COMMENT}, |
| {ADEID_FILEDATESI, ADEDOFF_FILEDATESI_EA, ADEDLEN_FILEDATESI}, |
| {ADEID_AFPFILEI, ADEDOFF_AFPFILEI_EA, ADEDLEN_AFPFILEI}, |
| - {ADEID_PRIVDEV, ADEDOFF_PRIVDEV_EA, ADEDLEN_INIT}, |
| - {ADEID_PRIVINO, ADEDOFF_PRIVINO_EA, ADEDLEN_INIT}, |
| - {ADEID_PRIVSYN, ADEDOFF_PRIVSYN_EA, ADEDLEN_INIT}, |
| - {ADEID_PRIVID, ADEDOFF_PRIVID_EA, ADEDLEN_INIT}, |
| + {ADEID_PRIVDEV, ADEDOFF_PRIVDEV_EA, ADEDLEN_PRIVDEV}, |
| + {ADEID_PRIVINO, ADEDOFF_PRIVINO_EA, ADEDLEN_PRIVINO}, |
| + {ADEID_PRIVSYN, ADEDOFF_PRIVSYN_EA, ADEDLEN_PRIVSYN}, |
| + {ADEID_PRIVID, ADEDOFF_PRIVID_EA, ADEDLEN_PRIVID}, |
| {0, 0, 0} |
| }; |
| |
| @@ -360,15 +360,22 @@ static int new_ad_header(struct adouble |
| const struct entry *eid; |
| uint16_t ashort; |
| struct stat st; |
| + char *adp = NULL; |
| |
| LOG(log_debug, logtype_ad, "new_ad_header(\"%s\")", path); |
| |
| if (ad_init_offsets(ad) != 0) |
| return -1; |
| |
| + if (ad->valid_data_len == 0) { |
| + ad->valid_data_len = ad->ad_vers == AD_VERSION_EA ? AD_DATASZ_EA : AD_DATASZ2; |
| + } |
| + adp = ad_entry(ad, ADEID_FINDERI); |
| + AFP_ASSERT(adp != NULL); |
| + |
| /* set default creator/type fields */ |
| - memcpy(ad_entry(ad, ADEID_FINDERI) + FINDERINFO_FRTYPEOFF,"\0\0\0\0", 4); |
| - memcpy(ad_entry(ad, ADEID_FINDERI) + FINDERINFO_FRCREATOFF,"\0\0\0\0", 4); |
| + memcpy(adp + FINDERINFO_FRTYPEOFF,"\0\0\0\0", 4); |
| + memcpy(adp + FINDERINFO_FRCREATOFF,"\0\0\0\0", 4); |
| |
| /* make things invisible */ |
| if ((ad->ad_options & ADVOL_INVDOTS) |
| @@ -378,14 +385,16 @@ static int new_ad_header(struct adouble |
| ashort = htons(ATTRBIT_INVISIBLE); |
| ad_setattr(ad, ashort); |
| ashort = htons(FINDERINFO_INVISIBLE); |
| - memcpy(ad_entry(ad, ADEID_FINDERI) + FINDERINFO_FRFLAGOFF, &ashort, sizeof(ashort)); |
| + memcpy(adp + FINDERINFO_FRFLAGOFF, &ashort, sizeof(ashort)); |
| } |
| |
| /* put something sane in the date fields */ |
| if (stp == NULL) { |
| stp = &st; |
| - if (lstat(path, &st) != 0) |
| + if (lstat(path, &st) != 0) { |
| + ad->valid_data_len = 0; |
| return -1; |
| + } |
| } |
| ad_setdate(ad, AD_DATE_CREATE | AD_DATE_UNIX, stp->st_mtime); |
| ad_setdate(ad, AD_DATE_MODIFY | AD_DATE_UNIX, stp->st_mtime); |
| @@ -417,7 +426,7 @@ static int parse_entries(struct adouble |
| |
| if (!eid |
| || eid > ADEID_MAX |
| - || off >= valid_data_len |
| + || ((eid != ADEID_RFORK) && (off >= valid_data_len)) |
| || ((eid != ADEID_RFORK) && (off + len > valid_data_len))) |
| { |
| LOG(log_warning, logtype_ad, "parse_entries: bogus eid: %u, off: %u, len: %u", |
| @@ -782,16 +791,41 @@ static int ad_header_read_ea(const char |
| EC_FAIL; |
| } |
| |
| + /* |
| + * It is possible for AFP metadata to contain a zero-length |
| + * comment. This will cause ad_entry(ad, ADEID_COMMENT) to return NULL |
| + * but should not be treated as an error condition. |
| + * Since recent CVE fixes have introduced new behavior regarding |
| + * ad_entry() output. For now, we will AFP_ASSERT() in EC_CLEANUP to prevent |
| + * altering on-disk info. This does introduce an avenue to DOS |
| + * the netatalk server by locally writing garbage to the EA. At this |
| + * point, the outcome is an acceptable risk to prevent unintended |
| + * changes to metadata. |
| + */ |
| if (nentries != ADEID_NUM_EA |
| || !ad_entry(ad, ADEID_FINDERI) |
| +#if 0 |
| || !ad_entry(ad, ADEID_COMMENT) |
| +#endif |
| || !ad_entry(ad, ADEID_FILEDATESI) |
| || !ad_entry(ad, ADEID_AFPFILEI) |
| || !ad_entry(ad, ADEID_PRIVDEV) |
| || !ad_entry(ad, ADEID_PRIVINO) |
| || !ad_entry(ad, ADEID_PRIVSYN) |
| || !ad_entry(ad, ADEID_PRIVID)) { |
| - LOG(log_error, logtype_ad, "ad_header_read_ea(\"%s\"): invalid metadata EA", fullpathname(path)); |
| + LOG(log_error, logtype_ad, |
| + "ad_header_read_ea(\"%s\"): invalid metadata EA " |
| + "this is now being treated as a fatal error. " |
| + "if you see this log entry, please file a bug ticket " |
| + "with your upstream vendor and attach the generated " |
| + "core file.", path ? fullpathname(path) : "UNKNOWN"); |
| + |
| + errno = EINVAL; |
| + EC_FAIL; |
| + } |
| + |
| + if (!ad_entry(ad, ADEID_COMMENT) && |
| + (ad->ad_eid[ADEID_COMMENT].ade_len != 0)) { |
| errno = EINVAL; |
| EC_FAIL; |
| } |
| @@ -805,6 +839,8 @@ static int ad_header_read_ea(const char |
| #endif |
| |
| EC_CLEANUP: |
| + AFP_ASSERT(!(ret != 0 && errno == EINVAL)); |
| +#if 0 |
| if (ret != 0 && errno == EINVAL) { |
| become_root(); |
| (void)sys_removexattr(path, AD_EA_META); |
| @@ -812,6 +848,7 @@ EC_CLEANUP: |
| LOG(log_error, logtype_ad, "ad_header_read_ea(\"%s\"): deleted invalid metadata EA", fullpathname(path), nentries); |
| errno = ENOENT; |
| } |
| +#endif |
| EC_EXIT; |
| } |
| |