Horton

Re: Potential buffer overrun in spell.c's CheckAffix()

x4mmm@yandex-team.ru

View thread View on postgr.es
From: Andrey Borodin <x4mmm@yandex-team.ru>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Date: 2026-04-22T11:57:26Z
Lists: pgsql-bugs 

> On 21 Apr 2026, at 22:32, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> if (Affix->type == FF_SUFFIX)
> {
> + /* protect against buffer overrun */
> + if (len < Affix->replen || len >= 2 * MAXNORMLEN ||
> + len - Affix->replen + findlen >= 2 * MAXNORMLEN)
> + return NULL;
> +
> strcpy(newword, word);
> strcpy(newword + len - Affix->replen, Affix->find);
> if (baselen) /* store length of non-changed part of word */
> @@ -2112,11 +2139,16 @@ CheckAffix(const char *word, size_t len, AFFIX *Affix, int flagflags, char *neww
> }
> else
> {
> + /* protect against buffer overrun */
> + if (len < Affix->replen ||
> + findlen + len - Affix->replen >= 2 * MAXNORMLEN)
> + return NULL;

Is there a reason for an asymmetric check "len >= 2 * MAXNORMLEN ||”?
Both cases seem symmetrical and we could move it out of “if".


> On 22 Apr 2026, at 03:35, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> I chose to do this by silently truncating the input before it can
> overrun the buffer, using logic comparable to the existing logic in
> get_nextfield().  Certainly there's at least as good an argument for
> raising an error, but for now let's follow the existing precedent.

Is there a reason not to emit WARNING? The data is obviously suspicious…
Perhaps, there’s a reason, so maybe just document it then.

Both patches look good to me, AFAICT.


Best regards, Andrey Borodin.

Commits

  1. Prevent some buffer overruns in spell.c's parsing of affix files.

  2. Prevent buffer overrun in spell.c's CheckAffix().