CVE-2021-44142: libadouble: harden parsing code

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14914

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

diff --git a/selftest/knownfail.d/samba.unittests.adouble b/selftest/knownfail.d/samba.unittests.adouble
deleted file mode 100644 (file)
index 8b0314f..0000000
--- a/selftest/knownfail.d/samba.unittests.adouble
+++ /dev/null
@@ -1,3 +0,0 @@
-^samba.unittests.adouble.parse_abouble_finderinfo2\(none\)
-^samba.unittests.adouble.parse_abouble_finderinfo3\(none\)
-^samba.unittests.adouble.parse_abouble_date2\(none\)

diff --git a/source3/lib/adouble.c b/source3/lib/adouble.c
index 6cbe8a5aeda77958c3e110b791fc5ccbdd5ccfcb..37fb686f17bb9e15c62330cb06cc2455897dd924 100644 (file)
--- a/source3/lib/adouble.c
+++ b/source3/lib/adouble.c
@@ -269,6 +269,95 @@ size_t ad_setentryoff(struct adouble *ad, int eid, size_t off)
        return ad->ad_eid[eid].ade_off = off;
 }
 
+/*
+ * 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},
+       };
+
+       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;
+               }
+       } 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;
+                       }
+               } else {
+                       if (got_len > ad_checks[eid].expected_len) {
+                               /* Too big for variable sized entry. */
+                               return false;
+                       }
+               }
+       }
+       if (off + got_len < off) {
+               /* wrap around */
+               return false;
+       }
+       if (off + got_len > bufsize) {
+               /* overflow */
+               return false;
+       }
+       return true;
+}
+
 /**
  * Return a pointer to an AppleDouble entry
  *
@@ -276,8 +365,15 @@ size_t ad_setentryoff(struct adouble *ad, int eid, size_t off)
  **/
 char *ad_get_entry(const struct adouble *ad, int eid)
 {
+       size_t bufsize = talloc_get_size(ad->ad_data);
        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;
@@ -914,20 +1010,11 @@ static bool ad_unpack(struct adouble *ad, const size_t nentries,
                        return false;
                }
 
-               /*
-                * 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), but we will fixup that
-                * in ad_convert(). And the resource fork is never
-                * accessed directly by the ad_data buf (also see
-                * comment above) anyway.
-                */
-               if ((eid != ADEID_RFORK) &&
-                   (eid != ADEID_FINDERI) &&
-                   ((off + len) > bufsize)) {
-                       DEBUG(1, ("bogus eid %d: off: %" PRIu32 ", len: %" PRIu32 "\n",
-                                 eid, off, len));
+               ok = ad_entry_check_size(eid, bufsize, off, len);
+               if (!ok) {
+                       DBG_ERR("bogus eid [%"PRIu32"] bufsize [%zu] "
+                               "off [%"PRIu32"] len [%"PRIu32"]\n",
+                               eid, bufsize, off, len);
                        return false;
                }