Fix CVE-2022-23121, CVE-2022-23123 regression

- Added guard check before access ad_entry()
- Allow zero length entry, for AppleDouble specification

etc/afpd/desktop.c

@@ -859,7 +859,7 @@ static int ad_addcomment(const AFPObj *obj, struct vol *vol, struct path *path,
         return( AFPERR_ACCESS );
     }
 
-    if (ad_getentryoff(adp, ADEID_COMMENT)) {
+    if (ad_getentryoff(adp, ADEID_COMMENT) && ad_entry(adp, ADEID_COMMENT)) {
         if ( (ad_get_MD_flags( adp ) & O_CREAT) ) {
             if ( *path->m_name == '\0' ) {
                 name = (char *)curdir->d_m_name->data;
@@ -932,7 +932,7 @@ static int ad_getcomment(struct vol *vol, struct path *path, char *rbuf, size_t
         return( AFPERR_NOITEM );
     }
 
-    if (!ad_getentryoff(adp, ADEID_COMMENT)) {
+    if (!ad_getentryoff(adp, ADEID_COMMENT) || !ad_entry(adp, ADEID_COMMENT)) {
         ad_close(adp, ADFLAGS_HF);
         return AFPERR_NOITEM;
     }

etc/afpd/directory.c

@@ -1519,10 +1519,7 @@ int getdirparams(const AFPObj *obj,
             break;
 
         case DIRPBIT_FINFO :
-            if ( isad ) {
-                ade = ad_entry(&ad, ADEID_FINDERI);
-                AFP_ASSERT(ade != NULL);
-
+            if ( isad && (ade = ad_entry(&ad, ADEID_FINDERI)) != NULL) {
                 memcpy( data, ade, 32 );
             } else { /* no appledouble */
                 memset( data, 0, 32 );
@@ -1902,15 +1899,13 @@ int setdirparams(struct vol *vol, struct path *path, uint16_t d_bitmap, char *bu
             }
             break;
         case DIRPBIT_FINFO :
-            if (isad) {
+            if (isad && (ade = ad_entry(&ad, ADEID_FINDERI)) != NULL) {
                 /* Fixes #2802236 */
                 uint16_t fflags;
                 memcpy(&fflags, finder_buf + FINDERINFO_FRFLAGOFF, sizeof(uint16_t));
                 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 ) {
                     /*

etc/afpd/extattrs.c

@@ -146,6 +146,7 @@ int afp_listextattr(AFPObj *obj _U_, char *ibuf, size_t ibuflen _U_, char *rbuf,
 
     adp = &ad;
     ad_init(adp, vol);
+    ad_init_offsets(adp);
 
     if (path_isadir(s_path)) {
 	    LOG(log_debug, logtype_afpd, "afp_listextattr(%s): is a dir", uname);
@@ -175,7 +176,7 @@ int afp_listextattr(AFPObj *obj _U_, char *ibuf, size_t ibuflen _U_, char *rbuf,
         close_ad = true;
         FinderInfo = ad_entry(adp, ADEID_FINDERI);
         /* Check if FinderInfo equals default and empty FinderInfo*/
-        if (memcmp(FinderInfo, emptyFinderInfo, 32) != 0) {
+        if (FinderInfo && memcmp(FinderInfo, emptyFinderInfo, 32) != 0) {
             /* FinderInfo contains some non 0 bytes -> include "com.apple.FinderInfo" */
             strcpy(attrnamebuf, ea_finderinfo);
             attrbuflen += strlen(ea_finderinfo) + 1;

etc/afpd/file.c

@@ -497,10 +497,7 @@ int getmetadata(const AFPObj *obj,
                 data += sizeof( aint );
             }
             else {
-                if ( adp ) {
-                    ade = ad_entry(adp, ADEID_FINDERI);
-                    AFP_ASSERT(ade != NULL);
-
+                if ( adp && (ade = ad_entry(adp, ADEID_FINDERI)) != NULL) {
                     memcpy(fdType, ade, 4);
 
                     if ( memcmp( fdType, "TEXT", 4 ) == 0 ) {
@@ -588,10 +585,8 @@ int getmetadata(const AFPObj *obj,
              * 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);
-
+            if (adp && (ad_meta_fileno(adp) != AD_SYMLINK)
+                && (ade = ad_entry(adp, ADEID_FINDERI)) != NULL) {
 	        memcpy(fdType, ade, 4);
                 if ( memcmp( fdType, "slnk", 4 ) == 0 ) {
 	 	    aint |= S_IFLNK;
@@ -1071,7 +1066,10 @@ int setfilparams(const AFPObj *obj, struct vol *vol,
                 break;
             }
             ade = ad_entry(adp, ADEID_FINDERI);
-            AFP_ASSERT(ade != NULL);
+            if (ade == NULL) {
+                LOG(log_debug, logtype_afpd, "setfilparams(\"%s\"): invalid FinderInfo", path->u_name);
+                break;
+            }
             if (default_type(ade)
                     && ( 
                      ((em = getextmap( path->m_name )) &&
@@ -1094,12 +1092,11 @@ int setfilparams(const AFPObj *obj, struct vol *vol,
             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(ade, fdType, 4 );
-                memcpy(ade + 4, "pdos", 4 );
+                if ((ade = ad_entry(adp, ADEID_FINDERI)) != NULL) {
+                    memcpy(ade, fdType, 4 );
+                    memcpy(ade + 4, "pdos", 4 );
+                }
                 break;
             }
             /* fallthrough */

etc/afpd/volume.c

@@ -330,11 +330,8 @@ static int getvolparams(const AFPObj *obj, uint16_t bitmap, struct vol *vol, str
             slash++;
         else
             slash = vol->v_path;
-        if (ad_getentryoff(&ad, ADEID_NAME)) {
+        if (ad_getentryoff(&ad, ADEID_NAME) && (ade = ad_entry(&ad, ADEID_NAME)) != NULL) {
             ad_setentrylen( &ad, ADEID_NAME, strlen( slash ));
-            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);

libatalk/adouble/ad_attr.c

@@ -8,6 +8,7 @@
 #include <atalk/util.h>
 #include <atalk/adouble.h>
 #include <atalk/logger.h>
+#include <atalk/errchk.h>
 
 #define FILEIOFF_ATTR 14
 #define AFPFILEIOFF_ATTR 2
@@ -21,20 +22,19 @@
 int ad_getattr(const struct adouble *ad, uint16_t *attr)
 {
     uint16_t fflags;
+    char *adp = NULL;
     *attr = 0;
 
-    if (ad_getentryoff(ad, ADEID_AFPFILEI)) {
-        char *adp = NULL;
-
-        adp = ad_entry(ad, ADEID_AFPFILEI);
-        AFP_ASSERT(adp != NULL);
+    if (ad_getentryoff(ad, ADEID_AFPFILEI) && (adp = ad_entry(ad, ADEID_AFPFILEI)) != NULL) {
         memcpy(attr, adp + AFPFILEIOFF_ATTR, 2);
 
         /* Now get opaque flags from FinderInfo */
-        adp = ad_entry(ad, ADEID_FINDERI);
-        AFP_ASSERT(adp != NULL);
-        memcpy(&fflags, adp + FINDERINFO_FRFLAGOFF, 2);
-
+        if ((adp = ad_entry(ad, ADEID_FINDERI)) != NULL) {
+            memcpy(&fflags, adp + FINDERINFO_FRFLAGOFF, 2);
+        } else {
+            LOG(log_debug, logtype_default, "ad_getattr(%s): invalid FinderInfo", ad->ad_name);
+            memset(&fflags, 0, 2);
+        }
         if (fflags & htons(FINDERINFO_INVISIBLE))
             *attr |= htons(ATTRBIT_INVISIBLE);
         else
@@ -60,6 +60,7 @@ int ad_getattr(const struct adouble *ad, uint16_t *attr)
 int ad_setattr(const struct adouble *ad, const uint16_t attribute)
 {
     uint16_t fflags;
+    char *ade = NULL, *adp = NULL;
 
     /* we don't save open forks indicator */
     uint16_t attr = attribute & ~htons(ATTRBIT_DOPEN | ATTRBIT_ROPEN);
@@ -69,13 +70,9 @@ int ad_setattr(const struct adouble *ad, const uint16_t attribute)
     if (ad->ad_adflags & ADFLAGS_DIR)
         attr &= ~(ATTRBIT_MULTIUSER | ATTRBIT_NOWRITE | ATTRBIT_NOCOPY);
 
-    if (ad_getentryoff(ad, ADEID_AFPFILEI) && ad_getentryoff(ad, ADEID_FINDERI)) {
-        char *adp = NULL;
-
-        adp = ad_entry(ad, ADEID_FINDERI);
-        AFP_ASSERT(adp != NULL);
-
-        memcpy(adp + AFPFILEIOFF_ATTR, &attr, sizeof(attr));
+    if (ad_getentryoff(ad, ADEID_AFPFILEI) && (ade = ad_entry(ad, ADEID_AFPFILEI)) != NULL
+        && ad_getentryoff(ad, ADEID_FINDERI) && (adp = ad_entry(ad, ADEID_FINDERI)) != NULL) {
+        memcpy(ade + AFPFILEIOFF_ATTR, &attr, sizeof(attr));
 
         /* Now set opaque flags in FinderInfo too */
         memcpy(&fflags, adp + FINDERINFO_FRFLAGOFF, 2);
@@ -104,26 +101,30 @@ int ad_setattr(const struct adouble *ad, const uint16_t attribute)
  */
 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)
 {
+    EC_INIT;
     uint32_t tmp;
     char *ade = NULL;
+    ssize_t id_len = -1, dev_len = -1, ino_len = -1, did_len = -1, syn_len = -1;
 
+    id_len = ad_getentrylen(adp, ADEID_PRIVID);
     ad_setentrylen( adp, ADEID_PRIVID, sizeof(id));
     tmp = id;
     if (adp->ad_vers == AD_VERSION_EA)
         tmp = htonl(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;
+        LOG(log_warning, logtype_ad, "ad_setid(%s): failed to set ADEID_PRIVID", adp->ad_name);
+        EC_FAIL;
     }
     memcpy(ade, &tmp, sizeof(tmp));
 
+    dev_len = ad_getentrylen(adp, ADEID_PRIVDEV);
     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;
+        LOG(log_warning, logtype_ad, "ad_setid(%s): failed to set ADEID_PRIVDEV", adp->ad_name);
+        EC_FAIL;
     }
 
     if ((adp->ad_options & ADVOL_NODEV)) {
@@ -132,34 +133,46 @@ int ad_setid (struct adouble *adp, const dev_t dev, const ino_t ino , const uint
         memcpy(ade, &dev, sizeof(dev_t));
     }
 
+    ino_len = ad_getentrylen(adp, ADEID_PRIVINO);
     ad_setentrylen( adp, ADEID_PRIVINO, sizeof(ino_t));
-
     ade = ad_entry(adp, ADEID_PRIVINO);
     if (ade == NULL) {
-        LOG(log_warning, logtype_ad, "ad_setid: failed to set ADEID_PRIVINO\n");
-        return -1;
+        LOG(log_warning, logtype_ad, "ad_setid(%s): failed to set ADEID_PRIVINO", adp->ad_name);
+        EC_FAIL;
     }
     memcpy(ade, &ino, sizeof(ino_t));
 
     if (adp->ad_vers != AD_VERSION_EA) {
+        did_len = ad_getentrylen(adp, ADEID_DID);
         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;
+            LOG(log_warning, logtype_ad, "ad_setid(%s): failed to set ADEID_DID", adp->ad_name);
+            EC_FAIL;
         }
         memcpy(ade, &did, sizeof(did));
     }
 
+    syn_len = ad_getentrylen(adp, ADEID_PRIVSYN);
     ad_setentrylen( adp, ADEID_PRIVSYN, 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;
+        LOG(log_warning, logtype_ad, "ad_setid(%s): failed to set ADEID_PRIVSYN", adp->ad_name);
+        EC_FAIL;
     }
     memcpy(ade, stamp, ADEDLEN_PRIVSYN);
 
+EC_CLEANUP:
+    if (ret != 0) {
+        if (id_len != -1) ad_setentrylen( adp, ADEID_PRIVID, id_len);
+        if (dev_len != -1) ad_setentrylen( adp, ADEID_PRIVDEV, dev_len);
+        if (ino_len != -1) ad_setentrylen( adp, ADEID_PRIVINO, ino_len);
+        if (did_len != -1) ad_setentrylen( adp, ADEID_DID, did_len);
+        if (syn_len != -1) ad_setentrylen( adp, ADEID_PRIVSYN, syn_len);
+        return 0;
+    }
+
     return 1;
 }
 

libatalk/adouble/ad_date.c

@@ -16,7 +16,7 @@ int ad_setdate(struct adouble *ad,
     if (xlate)
         date = AD_DATE_FROM_UNIX(date);
 
-    if (!ad_getentryoff(ad, ADEID_FILEDATESI))
+    if (!ad_getentryoff(ad, ADEID_FILEDATESI) || !ad_entry(ad, ADEID_FILEDATESI))
         return -1;
 
     if (dateoff > AD_DATE_ACCESS)
@@ -38,7 +38,7 @@ int ad_getdate(const struct adouble *ad,
     char *ade = NULL;
 
     dateoff &= AD_DATE_MASK;
-    if (!ad_getentryoff(ad, ADEID_FILEDATESI))
+    if (!ad_getentryoff(ad, ADEID_FILEDATESI) || !ad_entry(ad, ADEID_FILEDATESI))
         return -1;
 
     if (dateoff > AD_DATE_ACCESS)

libatalk/adouble/ad_flush.c

@@ -187,9 +187,11 @@ int ad_rebuild_adouble_header_osx(struct adouble *ad, char *adbuf)
     buf += sizeof( temp );
 
     ade = ad_entry(ad, ADEID_FINDERI);
-    AFP_ASSERT(ade != NULL);
-
-    memcpy(adbuf + ADEDOFF_FINDERI_OSX, ade, ADEDLEN_FINDERI);
+    if (ade != NULL) {
+        memcpy(adbuf + ADEDOFF_FINDERI_OSX, ade, ADEDLEN_FINDERI);
+    } else {
+        LOG(log_debug, logtype_ad, "ad_rebuild_adouble_header_osx(%s): invalid FinderInfo", ad->ad_name);
+    }
 
     /* rfork */
     temp = htonl( EID_DISK(ADEID_RFORK) );
@@ -219,6 +221,10 @@ int ad_copy_header(struct adouble *add, struct adouble *ads)
     char *src = NULL;
     char *dst = NULL;
 
+    if (add->valid_data_len == 0) {
+        LOG(log_error, logtype_ad, "ad_copy_header(%s): dst invalid valid_data_len", add->ad_name);
+        return -1;
+    }
     for ( eid = 0; eid < ADEID_MAX; eid++ ) {
         src = dst = NULL;
 
@@ -234,13 +240,15 @@ int ad_copy_header(struct adouble *add, struct adouble *ads)
         case ADEID_RFORK:
             continue;
         default:
+            if ((src = ad_entry(ads, eid)) == NULL) {
+                LOG(log_debug, logtype_ad, "ad_copy_header(%s): invalid src eid[%d]", ads->ad_name, eid);
+                continue;
+            }
+            if ((dst = ad_entry(add, eid)) == NULL) {
+                LOG(log_debug, logtype_ad, "ad_copy_header(%s): invalid dst eid[%d]", add->ad_name, eid);
+                continue;
+            }
             ad_setentrylen( add, eid, len );
-            dst = ad_entry(add, eid);
-            AFP_ASSERT(dst != NULL);
-
-            src = ad_entry(ads, eid);
-            AFP_ASSERT(src != NULL);
-
             memcpy( dst, src, len );
         }
     }
@@ -252,11 +260,13 @@ int ad_copy_header(struct adouble *add, struct adouble *ads)
         cnid_t id;
 
         dst = ad_entry(add, ADEID_PRIVID);
-        AFP_ASSERT(dst != NULL);
-
-        memcpy(&id, dst, sizeof(cnid_t));
-        id = htonl(id);
-        memcpy(dst, &id, sizeof(cnid_t));
+        if (dst != NULL) {
+            memcpy(&id, dst, sizeof(cnid_t));
+            id = htonl(id);
+            memcpy(dst, &id, sizeof(cnid_t));
+        } else {
+            LOG(log_debug, logtype_ad, "ad_copy_header(%s): invalid PRIVID", add->ad_name);
+        }
     }
     return 0;
 }