Skip to content
Snippets Groups Projects
Verified Commit 01970c58 authored by Simon Borek's avatar Simon Borek
Browse files

patches/packages: netatalk: backport segfaults fix: accepted

parent 7daee52c
No related branches found
No related tags found
No related merge requests found
Pipeline #100206 failed
From 1f7164ea833b5e7ff7cac5ad10eeecf231252058 Mon Sep 17 00:00:00 2001
From: Josef Schlehofer <pepe.schlehofer@gmail.com>
Date: Mon, 6 Jun 2022 13:36:14 +0200
Subject: [PATCH 1/2] Revert "Revert "netatalk: update to version 3.1.13""
This can be finally re-reverted, so we can use version 3.1.13, which
fixes multiple security vulnerabilities, but it segfaults almost
immediately. There is currently pending pull request, which fixes this,
and multiple users confirmed that it works on different GNU/Linux distributions.
This reverts commit bfe255064eeed30d06cbd969e4be36a89d76d0eb.
Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
---
net/netatalk/Makefile | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/netatalk/Makefile b/net/netatalk/Makefile
index 7b561d437..91e404ed8 100644
--- a/net/netatalk/Makefile
+++ b/net/netatalk/Makefile
@@ -8,12 +8,12 @@
include $(TOPDIR)/rules.mk
PKG_NAME:=netatalk
-PKG_VERSION:=3.1.12
-PKG_RELEASE:=2
+PKG_VERSION:=3.1.13
+PKG_RELEASE:=1
PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.bz2
PKG_SOURCE_URL:=@SF/netatalk
-PKG_HASH:=1560f83a3da41be97e0b70a96e2402159b8ddc631d38538360b14784beada5d1
+PKG_HASH:=89ada6bcfe1b39ad94f58c236654d1d944f2645c3e7de98b3374e0bd37d5e05d
PKG_BUILD_PARALLEL:=1
PKG_INSTALL:=1
--
2.34.1
From cc81050c660e115a8406f08b1da9b8913038287c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=A0imon=20Bo=C5=99ek?= <simon.borek@nic.cz>
Date: Thu, 28 Apr 2022 17:31:09 +0200
Subject: [PATCH 2/2] netatalk: backport pending PR to fix segfaults
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This commit backports pending PR, which solves segfaults:
- https://github.com/Netatalk/Netatalk/pull/174
To fix issues with segfaults described here:
- https://github.com/openwrt/packages/issues/18571
- https://github.com/Netatalk/Netatalk/issues/175
Signed-off-by: Šimon Bořek <simon.borek@nic.cz>
(cherry picked from commit ab768578cd06364cc9327a1718631d16e8aa3e20)
---
net/netatalk/Makefile | 2 +-
net/netatalk/patches/100-fix-segfaults.patch | 795 +++++++++++++++++++
2 files changed, 796 insertions(+), 1 deletion(-)
create mode 100644 net/netatalk/patches/100-fix-segfaults.patch
diff --git a/net/netatalk/Makefile b/net/netatalk/Makefile
index 91e404ed8..967b7adc3 100644
--- a/net/netatalk/Makefile
+++ b/net/netatalk/Makefile
@@ -9,7 +9,7 @@ include $(TOPDIR)/rules.mk
PKG_NAME:=netatalk
PKG_VERSION:=3.1.13
-PKG_RELEASE:=1
+PKG_RELEASE:=2
PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.bz2
PKG_SOURCE_URL:=@SF/netatalk
diff --git a/net/netatalk/patches/100-fix-segfaults.patch b/net/netatalk/patches/100-fix-segfaults.patch
new file mode 100644
index 000000000..0f8b03fe9
--- /dev/null
+++ b/net/netatalk/patches/100-fix-segfaults.patch
@@ -0,0 +1,795 @@
+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;
+ }
+
--
2.34.1
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment