From 751d7696646975f34c1d053a90c49756e7a87c68 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 13 Jan 2022 17:03:02 +0100 Subject: [PATCH] CVE-2021-44142: libadouble: harden parsing code BUG: https://bugzilla.samba.org/show_bug.cgi?id=14914 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison --- selftest/knownfail.d/samba.unittests.adouble | 3 - source3/lib/adouble.c | 115 ++++++++++++++++--- 2 files changed, 101 insertions(+), 17 deletions(-) delete mode 100644 selftest/knownfail.d/samba.unittests.adouble diff --git a/selftest/knownfail.d/samba.unittests.adouble b/selftest/knownfail.d/samba.unittests.adouble deleted file mode 100644 index 8b0314f2faec..000000000000 --- 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 6cbe8a5aeda7..37fb686f17bb 100644 --- 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; }