[PATCH] snowball: fix potential NULL dereference

Started by Коротков Максим11 months ago4 messages
#1Коротков Максим
m.korotkov@postgrespro.ru
1 attachment(s)

Hi all,
I found the case of potential NULL pointer dereference.
In snowball/libstemmer/api.c if we transfer control to the SN_create_env() function
by using the error label when there is a memory allocation error of z->p or z->S,
we can then dereference the NULL pointer z->S in the function SN_close_env().
Added the pointer check for avoiding a potential problem.
---
Best regards, Korotkov Maksim
PostgresPro
m.korotkov@postgrespro.ru

Attachments:

0001-snowball-fix-potential-NULL-dereference.patchtext/x-patchDownload
From c4596f6e23c4f5e9ae8294d51783afb733eccc9d Mon Sep 17 00:00:00 2001
From: Maksim Korotkov <m.korotkov@postgrespro.ru>
Date: Tue, 24 Dec 2024 17:07:12 +0300
Subject: [PATCH] snowball: fix potential NULL dereference

If we transfer control to the SN_create_env() function by using the error
label when there is a memory allocation error of z->p or z->S,
we can then dereference the NULL pointer z->S in the function SN_close_env().
Added the pointer check for avoiding potential problem.

Fixes: 140d4ebcb4 ("Tsearch2 functionality migrates to core.  The bulk of this work is by Oleg Bartunov and Teodor Sigaev, but I did a lot of editorializing, so anything that's broken is probably my fault.")

Found by Postgres Pro with Svace static analyzer
Signed-off-by: Maksim Korotkov <m.korotkov@postgrespro.ru>
---
 src/backend/snowball/libstemmer/api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/snowball/libstemmer/api.c b/src/backend/snowball/libstemmer/api.c
index 375938e6d1..3a7169abd8 100644
--- a/src/backend/snowball/libstemmer/api.c
+++ b/src/backend/snowball/libstemmer/api.c
@@ -34,7 +34,7 @@ error:
 extern void SN_close_env(struct SN_env * z, int S_size)
 {
     if (z == NULL) return;
-    if (S_size)
+    if (S_size && z->S)
     {
         int i;
         for (i = 0; i < S_size; i++)
-- 
2.34.1

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Коротков Максим (#1)
Re: [PATCH] snowball: fix potential NULL dereference

=?utf-8?q?=D0=9A=D0=BE=D1=80=D0=BE=D1=82=D0=BA=D0=BE=D0=B2_=D0=9C=D0=B0=D0=BA=D1=81=D0=B8=D0=BC?= <m.korotkov@postgrespro.ru> writes:

I found the case of potential NULL pointer dereference.
In snowball/libstemmer/api.c if we transfer control to the SN_create_env() function
by using the error label when there is a memory allocation error of z->p or z->S,
we can then dereference the NULL pointer z->S in the function SN_close_env().
Added the pointer check for avoiding a potential problem.

I believe you are right: OOM partway through SN_create_env would fail.
However, backend/snowball is not our code so applying our own local
patch is not the way to fix it. You should report this upstream;
see src/backend/snowball/README.

(Whenever they apply the patch, we should then re-sync...)

regards, tom lane

#3Maksim Korotkov
m.korotkov@postgrespro.ru
In reply to: Tom Lane (#2)
Re: [PATCH] snowball: fix potential NULL dereference

Hi
On 2025-02-11 19:53, Tom Lane wrote:

You should report this upstream;
(Whenever they apply the patch, we should then re-sync...)

FYI, the patch to fix this problem was applied by upstream.
---
Best regards, Korotkov Maksim
PostgresPro
m.korotkov@postgrespro.ru

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Maksim Korotkov (#3)
Re: [PATCH] snowball: fix potential NULL dereference

Maksim Korotkov <m.korotkov@postgrespro.ru> writes:

On 2025-02-11 19:53, Tom Lane wrote:

You should report this upstream;
(Whenever they apply the patch, we should then re-sync...)

FYI, the patch to fix this problem was applied by upstream.

Thanks. Barring objections, I'll do a full resync per
src/backend/snowball/README on HEAD, and then extract just
the null-dereference fix to apply to our back branches.

regards, tom lane