CVE-2022-23123: libatalk: harden ad_entry()

Also fixes: - CVE-2022-23122 - CVE-2022-23124 - CVE-2022-0194 Signed-off-by: Ralph Boehme <[email protected]>
rdmark · Apr 12, 2022 · 06ba3b5 · 06ba3b5
1 parent af38c35
commit 06ba3b5
Show file tree

Hide file tree

Showing 2 changed files with 119 additions and 1 deletion.
diff --git 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
@@ -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:
  *