public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tom Lane <[email protected]>
To: Andrey Borodin <[email protected]>
Cc: PostgreSQL mailing lists <[email protected]>
Subject: Re: Potential buffer overrun in spell.c's CheckAffix()
Date: Wed, 22 Apr 2026 10:50:09 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
I wrote:
> I suppose we could replace the strcpy with
> memcpy(newword, word, len - Affix->replen);
> and then we would not need the "len >= 2 * MAXNORMLEN" test
> and both paths could share the same check. There's something
> to be said for that, though it would be changing the logic to
> a greater extent than just "add some safety checks".
Concretely, about like this, where I also tried to make the actual
byte-copying steps a bit more uniform.
regards, tom lane
Attachments:
[text/x-diff] v2-0001-Prevent-buffer-overrun-in-spell.c-s-CheckAffix.patch (4.0K, 2-v2-0001-Prevent-buffer-overrun-in-spell.c-s-CheckAffix.patch)
download | inline diff:
From 844bb90d49f78c44c6ed395d245ff8a500b16395 Mon Sep 17 00:00:00 2001
From: Tom Lane <[email protected]>
Date: Wed, 22 Apr 2026 10:47:56 -0400
Subject: [PATCH v2] 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 a check 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.
Also make the actual copying steps a bit more readable, and remove
an unnecessary requirement for the whole input word to fit into the
output buffer (even though it always will with the current caller).
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 <[email protected]>
Reviewed-by: Andrey Borodin <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 14
---
src/backend/tsearch/spell.c | 47 ++++++++++++++++++++++++++++++++-----
1 file changed, 41 insertions(+), 6 deletions(-)
diff --git a/src/backend/tsearch/spell.c b/src/backend/tsearch/spell.c
index a1bfd2a9f9b..abaa18c21c4 100644
--- a/src/backend/tsearch/spell.c
+++ b/src/backend/tsearch/spell.c
@@ -2065,9 +2065,32 @@ 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.
+ * The transformation consists of replacing Affix->replen leading or
+ * trailing bytes with the Affix->find string.
+ *
+ * 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 keeplen,
+ findlen;
+
/*
* Check compound allow flags
*/
@@ -2100,15 +2123,27 @@ CheckAffix(const char *word, size_t len, AFFIX *Affix, int flagflags, char *neww
return NULL;
}
+ /*
+ * Protect against output buffer overrun (len < Affix->replen would be
+ * caller error, but check anyway)
+ */
+ Assert(len == strlen(word));
+ if (len < Affix->replen)
+ return NULL;
+ keeplen = len - Affix->replen; /* how much of word we will keep */
+ findlen = strlen(Affix->find);
+ if (keeplen + findlen >= 2 * MAXNORMLEN)
+ return NULL;
+
/*
* make replace pattern of affix
*/
if (Affix->type == FF_SUFFIX)
{
- strcpy(newword, word);
- strcpy(newword + len - Affix->replen, Affix->find);
+ memcpy(newword, word, keeplen);
+ strcpy(newword + keeplen, Affix->find);
if (baselen) /* store length of non-changed part of word */
- *baselen = len - Affix->replen;
+ *baselen = keeplen;
}
else
{
@@ -2116,10 +2151,10 @@ CheckAffix(const char *word, size_t len, AFFIX *Affix, int flagflags, char *neww
* 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);
+ memcpy(newword, Affix->find, findlen);
+ strcpy(newword + findlen, word + Affix->replen);
}
/*
--
2.43.7
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: Potential buffer overrun in spell.c's CheckAffix()
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox