Uninitialized var utilized (src/backend/tsearch/spell.c)

Started by Ranier Vilelaover 5 years ago5 messages
#1Ranier Vilela
ranier.vf@gmail.com
1 attachment(s)

At function NIImportAffixes (src/backend/tsearch/spell.c).

If option "flag" is not handled, variable char flag[BUFSIZE] will remain
uninitialized.

regards,
Ranier Vilela

Attachments:

fix_uninitialized_var_flag_spell.patchapplication/octet-stream; name=fix_uninitialized_var_flag_spell.patchDownload
diff --git a/src/backend/tsearch/spell.c b/src/backend/tsearch/spell.c
index 05d08cfc01..42fda514c3 100644
--- a/src/backend/tsearch/spell.c
+++ b/src/backend/tsearch/spell.c
@@ -1439,6 +1439,7 @@ NIImportAffixes(IspellDict *Conf, const char *filename)
 	Conf->usecompound = false;
 	Conf->useFlagAliases = false;
 	Conf->flagMode = FM_CHAR;
+	flag[0] = '\0';
 
 	while ((recoded = tsearch_readline(&trst)) != NULL)
 	{
#2Daniel Gustafsson
daniel@yesql.se
In reply to: Ranier Vilela (#1)
Re: Uninitialized var utilized (src/backend/tsearch/spell.c)

On 9 Oct 2020, at 14:36, Ranier Vilela <ranier.vf@gmail.com> wrote:

At function NIImportAffixes (src/backend/tsearch/spell.c).

If option "flag" is not handled, variable char flag[BUFSIZE] will remain uninitialized.

To help reviewers, your report should contain an explanation of when that can
happen.

cheers ./daniel

#3Ranier Vilela
ranier.vf@gmail.com
In reply to: Daniel Gustafsson (#2)
Re: Uninitialized var utilized (src/backend/tsearch/spell.c)

Em sex., 9 de out. de 2020 às 11:08, Daniel Gustafsson <daniel@yesql.se>
escreveu:

On 9 Oct 2020, at 14:36, Ranier Vilela <ranier.vf@gmail.com> wrote:

At function NIImportAffixes (src/backend/tsearch/spell.c).

If option "flag" is not handled, variable char flag[BUFSIZE] will remain

uninitialized.

To help reviewers, your report should contain an explanation of when that
can
happen.

When option "flag" is not handled.

if (STRNCMP(pstr, "flag") == 0)

regards,
Ranier Vilela

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Ranier Vilela (#3)
Re: Uninitialized var utilized (src/backend/tsearch/spell.c)

Ranier Vilela <ranier.vf@gmail.com> writes:

Em sex., 9 de out. de 2020 às 11:08, Daniel Gustafsson <daniel@yesql.se>
escreveu:

To help reviewers, your report should contain an explanation of when that
can happen.

When option "flag" is not handled.
if (STRNCMP(pstr, "flag") == 0)

I think what he means is that if the file contains no "flag" command
before an affix entry then then we would arrive at NIAddAffix with an
undefined flag buffer. That's illegal syntax according to a quick scan
of the ispell(5) man page, which explains the lack of complaints; but
it might be worth guarding against.

Aside from failing to initialize some variables that need it, it looks to
me like NIImportAffixes is uselessly initializing some variables that
don't need it. I'd also be inclined to figure out which values are
actually meant to be carried across lines, and declare the ones that
aren't inside the loop, just for clarity.

regards, tom lane

#5Ranier Vilela
ranier.vf@gmail.com
In reply to: Tom Lane (#4)
Re: Uninitialized var utilized (src/backend/tsearch/spell.c)

Em sex., 9 de out. de 2020 às 11:37, Tom Lane <tgl@sss.pgh.pa.us> escreveu:

Ranier Vilela <ranier.vf@gmail.com> writes:

Em sex., 9 de out. de 2020 às 11:08, Daniel Gustafsson <daniel@yesql.se>
escreveu:

To help reviewers, your report should contain an explanation of when

that

can happen.

When option "flag" is not handled.
if (STRNCMP(pstr, "flag") == 0)

I think what he means is that if the file contains no "flag" command
before an affix entry then then we would arrive at NIAddAffix with an
undefined flag buffer. That's illegal syntax according to a quick scan
of the ispell(5) man page, which explains the lack of complaints; but
it might be worth guarding against.

Aside from failing to initialize some variables that need it, it looks to
me like NIImportAffixes is uselessly initializing some variables that
don't need it. I'd also be inclined to figure out which values are
actually meant to be carried across lines, and declare the ones that
aren't inside the loop, just for clarity.

Thanks Tom, for the great explanation.

regards,
Ranier Vilela