Re: replace strtok()

Em ter., 15 de out. de 2024 às 03:45, Peter Eisentraut <peter(at)eisentraut(dot)org>
escreveu:

> On 10.10.24 14:59, Ranier Vilela wrote:
> > Please look at the SCRAM secret, which breaks parse_scram_secret(),
> > perhaps because strsep() doesn't return NULL where strtok() did:
> >
> > CREATE ROLE r PASSWORD
> > 'SCRAM-
> >
> SHA-256$4096:hpFyHTUsSWcR7O9P$LgZFIt6Oqdo27ZFKbZ2nV+=vtnYM995pDh9ca6WSi120qVV5NeluNfUPkwm7Vqat25RjSPLkGeoZBQs6wVv+um4=';
> >
> > Core was generated by `postgres: law regression [local] CREATE
> > ROLE '.
> > Program terminated with signal SIGSEGV, Segmentation fault.
> >
> > #0 __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:74
> > (gdb) bt
> > #0 __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:74
> > #1 0x0000563625e9e5b0 in parse_scram_secret (...) at
> auth-scram.c:655
> >
> > Thanks for the report.
> >
> > It seems to me that it could be due to incorrect use of the strsep
> function.
> > See:
> > https://man7.org/linux/man-pages/man3/strsep.3.html <https://man7.org/
> > linux/man-pages/man3/strsep.3.html>
> > "
> >
> > In case no delimiter was found, the token
> > is taken to be the entire string/*stringp/, and/*stringp/ is made
> > NULL.
> >
> > "
> > So, it is necessary to check the *stringp* against NULL too.
>
> Thanks for the analysis. I think moreover we *only* need to check the
> "stringp" for NULL, not the return value of strsep(), which would never
> be NULL in our case. So I propose the attached patch as a variant of
> yours.
>
I'm not 100% sure, but the contrib passwordcheck uses and It's not very
safe.
The checks of NULL return are cheap, and will protect unwary users.

So I'm neutral here.

Notice that the report is only by Alexander Lakhin.
I'm at most a reviewer here.

best regards,
Ranier Vilela