[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAHb2+K7diW05LBEu5Kfw5UoieN+OxMeb-SsY_pzspeDf2uzZHw@mail.gmail.com>
Date: Sun, 16 Feb 2025 21:05:47 +0300
From: Anton Moryakov <ant.v.moryakov@...il.com>
To: Rich Felker <dalias@...c.org>
Cc: Mike Granby <mikeg@...eg.net>, "musl@...ts.openwall.com" <musl@...ts.openwall.com>
Subject: Re: [PATCH] compat: time32: Fix potential NULL dereference in
thanks for the feedback. I checked again, yes, the static analyzer was wrong
вс, 16 февр. 2025 г. в 21:04, Rich Felker <dalias@...c.org>:
> On Sun, Feb 16, 2025 at 06:00:07PM +0000, Mike Granby wrote:
> > Doesn't this still pass a NULL when times32 is zero?
> >
> > In fact, isn't it just exactly equivalent?
>
> Yes, the dereference is already conditional on times32 being non-null.
>
> > In fact, isn't a bad a idea just to blindly post the output of static
> analysis tools?
>
> Especially if the static analysis tool does not understand that the
> untaken branch of the tertiary operator is not evaluated...
>
>
> > -----Original Message-----
> > From: Anton Moryakov <ant.v.moryakov@...il.com>
> > Sent: Sunday, February 16, 2025 12:56 PM
> > To: musl@...ts.openwall.com
> > Cc: Anton Moryakov <ant.v.moryakov@...il.com>
> > Subject: [musl] [PATCH] compat: time32: Fix potential NULL dereference in
> >
> > Static analyzer reported:
> > Pointer '0', which is passed as 2nd parameter in call to function
> '__lutimes_time64' at lutimes_time32.c:9, where it is dereferenced at
> lutimes.c:9, may have a NULL value.
> >
> > Corrections explained:
> > The function __lutimes_time32 could pass a NULL pointer to lutimes when
> times32 was NULL, leading to potential dereference of NULL inside lutimes.
> This has been fixed by explicitly handling the case where times32 is NULL
> and passing NULL directly to lutimes.
> >
> > Additionally, the conversion from struct timeval32 to struct timeval is
> now done in a separate block when times32 is not NULL, ensuring safe access
> to the times32 array.
> >
> > Triggers found by static analyzer Svace.
> >
> > Signed-off-by: Anton Moryakov <ant.v.moryakov@...il.com>
> >
> > ---
> > compat/time32/lutimes_time32.c | 14 ++++++++++----
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/compat/time32/lutimes_time32.c
> b/compat/time32/lutimes_time32.c index 7f75cd4a..48f839f6 100644
> > --- a/compat/time32/lutimes_time32.c
> > +++ b/compat/time32/lutimes_time32.c
> > @@ -6,7 +6,13 @@
> >
> > int __lutimes_time32(const char *path, const struct timeval32
> times32[2]) {
> > - return lutimes(path, !times32 ? 0 : ((struct timeval[2]){
> > - {.tv_sec = times32[0].tv_sec,.tv_usec =
> times32[0].tv_usec},
> > - {.tv_sec = times32[1].tv_sec,.tv_usec =
> times32[1].tv_usec}}));
> > -}
> > + if (!times32) {
> > + return lutimes(path, NULL);
> > + } else {
> > + struct timeval times[2] = {
> > + {.tv_sec = times32[0].tv_sec, .tv_usec =
> times32[0].tv_usec},
> > + {.tv_sec = times32[1].tv_sec, .tv_usec = times32[1].tv_usec}
> > + };
> > + return lutimes(path, times);
> > + }
> > +}
> > \ No newline at end of file
> > --
> > 2.30.2
>
Content of type "text/html" skipped
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
