Re: BUG #19595: Three memory-safety defects in src/backend/tsearch/spell.c (dictionary loader), PG 18.3 - Mailing list pgsql-bugs

The following bug has been logged on the website:

Bug reference:      19595
Logged by:          Michael Malis
Email address:      michaelmalis2@gmail.com
PostgreSQL version: 18.3
Operating system:   MacOS
Description:       

(I initially filed this at security@ but because the dictionary is
considered a trusted
file Tom asked me to repost here)

Three memory-safety defects in the ispell/hunspell dictionary loader, all
reached by CREATE TEXT SEARCH DICTIONARY on a malformed dictionary file.

BUG 1 -- out-of-bounds heap write in NISortAffixes()

  1987:  Conf->CompoundAffix = ptr = (CMPDAffix *)
palloc(sizeof(CMPDAffix) * Conf->naffixes);
  ...    /* loop over i < naffixes; ptr++ once per collected affix */
  2015:  ptr->affix = NULL;
  2016:  Conf->CompoundAffix = repalloc(Conf->CompoundAffix,
sizeof(CMPDAffix) * (ptr - Conf->CompoundAffix + 1));

The array holds exactly naffixes elements. When every affix is collected,
ptr == base + naffixes, so line 2015 writes one element (8 bytes) past the
end -- and the repalloc that would make room for the terminator is on the
next line, after the write. CMPDAffix is 16 bytes, palloc rounds to
power-of-two chunks, so the write escapes its chunk when naffixes is a power
of two, and always escapes for naffixes > 512 (dedicated block), which
covers typical dictionaries. The store then lands in the next chunk's header
or in malloc metadata.

Reproducer -- in $SHAREDIR/tsearch_data, oob.affix:

    compoundwords controlled Z
    suffixes
    flag ~Z:
        . > S

oob.dict:

    foo/Z

then:

    CREATE TEXT SEARCH DICTIONARY oob (TEMPLATE = ispell, DictFile =
oob, AffFile = oob);
    SELECT ts_lexize('oob', 'foos');

This gives naffixes == 1 with the one affix collected. The allocator detects
the damage at the next allocation ("free list is damaged", aborting in
palloc from mkANode from NISortAffixes).

Fix: allocate naffixes + 1 at line 1987, or write the terminator after the
repalloc.


BUG 2 -- uninitialized stack buffer read in NIImportAffixes()

  1426:  char        flag[BUFSIZ];      /* never initialized */
  ...
  1519:  flag[0] = *s++;                /* only written in the "flag" branch
*/
  1520:  flag[1] = '\0';
  ...
  1543:  NIAddAffix(Conf, flag, flagflags, mask, find, repl, ...);
/* unconditional */

flag is written only inside the "flag" directive branch but passed
unconditionally to NIAddAffix, which does cpstrdup(Conf, flag) -- strlen +
strcpy over uninitialized stack. If no "flag" line was parsed, this is an
unbounded strlen (no guaranteed NUL in BUFSIZ) and stack contents are copied
into a long-lived dictionary flag.

Trigger: a .affix file with an old-format "prefixes"/"suffixes" section and
a parseable affix entry but no "flag" directive, e.g.:

    COMPOUNDWORDS l 1
    suffixes
    nlag Z:
        . > S

with dict "foo/Z" ("nlag" is simply not "flag", so the directive is never
seen while the entry still parses).

Fix: initialize flag[0] = '\0' at declaration.


BUG 3 -- NULL dereference on unfilled AF alias slots

  1324:  Conf->AffixData = (const char **) palloc0(naffix * sizeof(char *));
  1336:  Conf->AffixData[curaffix] = cpstrdup(Conf, sflag);   /* one
per AF line */

The alias table is zero-filled and only the AF lines actually present are
filled. If "AF <n>" declares more slots than the file fills, the tail stays
NULL, and those NULL slots are dereferenced without a check:

  - MergeAffix() line 1576:  if (*Conf->AffixData[a1] == '\0')
    (the Assert at 1573 checks only the index, and asserts are off in
    production builds), reached from NISortDictionary() at line 1691;
  - getAffixFlagSet() (1156) -> getCompoundAffixFlagValue() (1120) ->
    getNextFlagFromString() (350), reached from inside NIImportOOAffixes().

Trigger: a .affix file whose AF table declares a larger count than the
number of AF lines it provides, with a dictionary word referencing an
unfilled index.

Fix: reject an incompletely populated AF table, or treat a NULL slot as the
empty flag set (VoidString) at the dereference sites.

Reachability: CREATE TEXT SEARCH DICTIONARY requires CREATE on a schema, not
superuser (DefineTSDictionary in src/backend/commands/tsearchcmds.c). The
file path is restricted to [a-z0-9_] under $SHAREDIR/tsearch_data
(get_tsearch_config_filename in src/backend/tsearch/ts_utils.c), so the
crafted file must be placed there by other means -- most realistically a
corrupt or hostile third-party hunspell dictionary. Minimized file pairs
available on request.