From 06ba3b54bba480349cb50cf3562f5b976e3f9937 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Thu, 10 Mar 2022 13:24:28 +0100
Subject: [PATCH] CVE-2022-23123: libatalk: harden ad_entry()
Also fixes:
- CVE-2022-23122
- CVE-2022-23124
- CVE-2022-0194
Signed-off-by: Ralph Boehme <slow@samba.org>
---
include/atalk/adouble.h | 3 +-
libatalk/adouble/ad_open.c | 117 +++++++++++++++++++++++++++++++++++++
2 files changed, 119 insertions(+), 1 deletion(-)
diff --git a/include/atalk/adouble.h b/include/atalk/adouble.h
index 16dd327d077e..da50b19ca460 100644
--- a/include/atalk/adouble.h
+++ b/include/atalk/adouble.h
@@ -258,6 +258,7 @@ struct adouble {
int ad_m_namelen;
struct adouble_fops *ad_ops;
u_int16_t ad_open_forks; /* open forks (by others) */
+ size_t valid_data_len; /* Bytes read into ad_data */
#ifdef USE_MMAPPED_HEADERS
char *ad_data;
@@ -416,7 +417,6 @@ struct adouble_fops {
#define ad_getentrylen(ad,eid) ((ad)->ad_eid[(eid)].ade_len)
#define ad_setentrylen(ad,eid,len) ((ad)->ad_eid[(eid)].ade_len = (len))
#define ad_getentryoff(ad,eid) ((ad)->ad_eid[(eid)].ade_off)
-#define ad_entry(ad,eid) ((caddr_t)(ad)->ad_data + (ad)->ad_eid[(eid)].ade_off)
#define ad_get_HF_flags(ad) ((ad)->ad_resource_fork.adf_flags)
#define ad_get_MD_flags(ad) ((ad)->ad_md->adf_flags)
@@ -448,6 +448,7 @@ extern int ad_excl_lock (struct adouble * /*adp*/, const u_int32_t /*eid*/);
#define ad_unlock ad_fcntl_unlock
/* ad_open.c */
+extern void *ad_entry(const struct adouble *ad, int eid);
extern int ad_setfuid (const uid_t );
extern uid_t ad_getfuid (void );
extern char *ad_dir (const char *);
diff --git a/libatalk/adouble/ad_open.c b/libatalk/adouble/ad_open.c
index 5f5a6b90330b..390e095cbd81 100644
--- a/libatalk/adouble/ad_open.c
+++ b/libatalk/adouble/ad_open.c
@@ -521,6 +521,7 @@ static int parse_entries(struct adouble *ad, uint16_t nentries,
}
}
+ ad->valid_data_len = valid_data_len;
return 0;
}
@@ -754,6 +755,122 @@ static int ad_header_sfm_read(struct adouble *ad, struct stat *hst)
return 0;
}
+/*
+ * All entries besides FinderInfo and resource fork must fit into the
+ * buffer. FinderInfo is special as it may be larger then the default 32 bytes
+ * if it contains marshalled xattrs, which we will fixup that in
+ * ad_convert(). The first 32 bytes however must also be part of the buffer.
+ *
+ * The resource fork is never accessed directly by the ad_data buf.
+ */
+static bool ad_entry_check_size(uint32_t eid,
+ size_t bufsize,
+ uint32_t off,
+ uint32_t got_len)
+{
+ struct {
+ off_t expected_len;
+ bool fixed_size;
+ bool minimum_size;
+ } ad_checks[] = {
+ [ADEID_DFORK] = {-1, false, false}, /* not applicable */
+ [ADEID_RFORK] = {-1, false, false}, /* no limit */
+ [ADEID_NAME] = {ADEDLEN_NAME, false, false},
+ [ADEID_COMMENT] = {ADEDLEN_COMMENT, false, false},
+ [ADEID_ICONBW] = {ADEDLEN_ICONBW, true, false},
+ [ADEID_ICONCOL] = {ADEDLEN_ICONCOL, false, false},
+ [ADEID_FILEI] = {ADEDLEN_FILEI, true, false},
+ [ADEID_FILEDATESI] = {ADEDLEN_FILEDATESI, true, false},
+ [ADEID_FINDERI] = {ADEDLEN_FINDERI, false, true},
+ [ADEID_MACFILEI] = {ADEDLEN_MACFILEI, true, false},
+ [ADEID_PRODOSFILEI] = {ADEDLEN_PRODOSFILEI, true, false},
+ [ADEID_MSDOSFILEI] = {ADEDLEN_MSDOSFILEI, true, false},
+ [ADEID_SHORTNAME] = {ADEDLEN_SHORTNAME, false, false},
+ [ADEID_AFPFILEI] = {ADEDLEN_AFPFILEI, true, false},
+ [ADEID_DID] = {ADEDLEN_DID, true, false},
+ [ADEID_PRIVDEV] = {ADEDLEN_PRIVDEV, true, false},
+ [ADEID_PRIVINO] = {ADEDLEN_PRIVINO, true, false},
+ [ADEID_PRIVSYN] = {ADEDLEN_PRIVSYN, true, false},
+ [ADEID_PRIVID] = {ADEDLEN_PRIVID, true, false},
+ };
+ uint32_t required_len;
+
+ if (eid >= ADEID_MAX) {
+ return false;
+ }
+ if (got_len == 0) {
+ /* Entry present, but empty, allow */
+ return true;
+ }
+ if (ad_checks[eid].expected_len == 0) {
+ /*
+ * Shouldn't happen: implicitly initialized to zero because
+ * explicit initializer missing.
+ */
+ return false;
+ }
+ if (ad_checks[eid].expected_len == -1) {
+ /* Unused or no limit */
+ return true;
+ }
+ if (ad_checks[eid].fixed_size) {
+ if (ad_checks[eid].expected_len != got_len) {
+ /* Wrong size fo fixed size entry. */
+ return false;
+ }
+ required_len = got_len;
+ } else {
+ if (ad_checks[eid].minimum_size) {
+ if (got_len < ad_checks[eid].expected_len) {
+ /*
+ * Too small for variable sized entry with
+ * minimum size.
+ */
+ return false;
+ }
+ required_len = got_len;
+ } else {
+ if (got_len > ad_checks[eid].expected_len) {
+ /* Too big for variable sized entry. */
+ return false;
+ }
+ /*
+ * Expect the buffer to provide enough room for the maximum possible
+ * size.
+ */
+ required_len = ad_checks[eid].expected_len;
+ }
+ }
+ if (off + required_len < off) {
+ /* wrap around */
+ return false;
+ }
+ if (off + required_len > bufsize) {
+ /* overflow */
+ return false;
+ }
+ return true;
+}
+
+void *ad_entry(const struct adouble *ad, int eid)
+{
+ size_t bufsize = ad->valid_data_len;
+ off_t off = ad_getentryoff(ad, eid);
+ size_t len = ad_getentrylen(ad, eid);
+ bool valid;
+
+ valid = ad_entry_check_size(eid, bufsize, off, len);
+ if (!valid) {
+ return NULL;
+ }
+
+ if (off == 0 || len == 0) {
+ return NULL;
+ }
+
+ return ((struct adouble *)ad)->ad_data + off;
+}
+
/* ---------------------------------------
* Put the .AppleDouble where it needs to be:
*