[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250216180419.GH10433@brightrain.aerifal.cx>
Date: Sun, 16 Feb 2025 13:04:19 -0500
From: Rich Felker <dalias@...c.org>
To: Mike Granby <mikeg@...eg.net>
Cc: "musl@...ts.openwall.com" <musl@...ts.openwall.com>,
Anton Moryakov <ant.v.moryakov@...il.com>
Subject: Re: [PATCH] compat: time32: Fix potential NULL dereference in
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
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.
