Potential buffer overrun in spell.c's CheckAffix() - Mailing list pgsql-bugs

From Tom Lane
Subject Potential buffer overrun in spell.c's CheckAffix()
Date
Msg-id 641711.1776792744@sss.pgh.pa.us
Whole thread
Responses Re: Potential buffer overrun in spell.c's CheckAffix()
Re: Potential buffer overrun in spell.c's CheckAffix()
List pgsql-bugs
CheckAffix is used by our ispell text search dictionaries to attach a
prefix or suffix to a given base word.  The input word is known to be
no longer than MAXNORMLEN (256), and an output buffer of size
MAXNORMLEN * 2 is provided.  But there's not any a-priori limit on the
length of a prefix or suffix string, so in principle a buffer overflow
could occur.

In practice these limits seem like more than plenty for any real-world
word, so I think it's sufficient to just reject the prefix or suffix
if an overflow would occur, as attached.

This bug was reported to pgsql-security by Xint Code as a potential
security issue.  However we decided it doesn't seem worth the CVE
treatment, because exploiting it would require getting a malicious
ispell dictionary installed in a PG server.  Putting the .dict file
into the installation's file tree would require superuser privileges,
and so would creating a text dictionary SQL object that references it.
Maybe an attacker could persuade a gullible DBA to do that, but there
are plenty of other attack pathways available if you're that
persuasive.

Despite that, it seems worth fixing as a run-of-the-mill bug.
Any objections to the attached?

            regards, tom lane

From 977c82d2501465789ccda052f0b718183e89d816 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun, 12 Apr 2026 20:42:15 -0400
Subject: [PATCH v1] Prevent buffer overrun in spell.c's CheckAffix().

This function writes into a caller-supplied buffer of length
2 * MAXNORMLEN, which should be plenty in real-world cases.
However a malicious affix file could supply an affix long
enough to overrun that.  Defend by just rejecting the match
if it would overrun the buffer.  I also inserted checks of
the input word length against Affix->replen, just to be sure
we won't index off the buffer, though it would be caller error
for that not to be true.

The lack of documentation in this code makes my head hurt, so
I also reverse-engineered a basic header comment for CheckAffix.

Reported-by: Xint Code
Author: Tom Lane <tgl@sss.pgh.pa.us>
Backpatch-through: 14
---
 src/backend/tsearch/spell.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/src/backend/tsearch/spell.c b/src/backend/tsearch/spell.c
index a1bfd2a9f9b..79a2a459406 100644
--- a/src/backend/tsearch/spell.c
+++ b/src/backend/tsearch/spell.c
@@ -2065,9 +2065,29 @@ FindAffixes(AffixNode *node, const char *word, int wrdlen, int *level, int type)
     return NULL;
 }

+/*
+ * Checks to see if affix applies to word, transforms word if so.
+ *
+ * word: input word
+ * len: length of input word
+ * Affix: affix to consider
+ * flagflags: context flags showing whether we are handling a compound word
+ * newword: output buffer (MUST be of length 2 * MAXNORMLEN)
+ * baselen: input/output argument
+ *
+ * If baselen isn't NULL, then *baselen is used to return the length of
+ * the non-changed part of the word when applying a suffix, and is used
+ * to detect whether the input contained only a prefix and suffix when
+ * later applying a prefix.
+ *
+ * Returns newword on success, or NULL if the affix can't be applied.
+ * On success, the modified word is stored into newword.
+ */
 static char *
 CheckAffix(const char *word, size_t len, AFFIX *Affix, int flagflags, char *newword, int *baselen)
 {
+    size_t        findlen;
+
     /*
      * Check compound allow flags
      */
@@ -2103,8 +2123,15 @@ CheckAffix(const char *word, size_t len, AFFIX *Affix, int flagflags, char *neww
     /*
      * make replace pattern of affix
      */
+    Assert(len == strlen(word));
+    findlen = strlen(Affix->find);
     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;
+
         /*
          * if prefix is an all non-changed part's length then all word
          * contains only prefix and suffix, so out
          */
-        if (baselen && *baselen + strlen(Affix->find) <= Affix->replen)
+        if (baselen && *baselen + findlen <= Affix->replen)
             return NULL;
         strcpy(newword, Affix->find);
         strcat(newword, word + Affix->replen);
--
2.43.7


pgsql-bugs by date:

Previous
From: Tom Lane
Date:
Subject: to_char() vs. long numeric formatting strings from locale
Next
From: Tom Lane
Date:
Subject: Re: Potential buffer overrun in spell.c's CheckAffix()