Difuse/packages
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

netatalk: backport pending PR to fix segfaults

Browse source 
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>
This commit is contained in:
Šimon Bořek 2022-04-28 17:31:09 +02:00 committed by Daniel Golle
parent 3d1254a6dc
commit 8ad1582871
2 changed files with 796 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
net/netatalk/Makefile
View file

@ -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

795
net/netatalk/patches/100-fix-segfaults.patch Normal file
View file

@ -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;
}