diff options
author | Ralph Boehme <slow@samba.org> | 2022-01-13 17:03:02 +0100 |
---|---|---|
committer | Jule Anger <janger@samba.org> | 2022-01-30 14:08:00 +0100 |
commit | edf661e40b3c17681563613bf58b2e757a6ebc12 (patch) | |
tree | 794d2c81f0f391299c530cde3afe0227a72ca42e | |
parent | ffa40d4acb5f2d57443670be31dca05caacb1cfd (diff) | |
download | samba-edf661e40b3c17681563613bf58b2e757a6ebc12.tar.gz |
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>
-rw-r--r-- | selftest/knownfail.d/samba.unittests.adouble | 3 | ||||
-rw-r--r-- | source3/lib/adouble.c | 115 |
2 files changed, 101 insertions, 17 deletions
diff --git a/selftest/knownfail.d/samba.unittests.adouble b/selftest/knownfail.d/samba.unittests.adouble deleted file mode 100644 index 8b0314f2fae..00000000000 --- 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 48621feb530..bb94a45e2de 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; } |