Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Sun, 24 Mar 2024 15:33:42 -0400
From: Rich Felker <dalias@...ifal.cx>
To: Maks Mishin <maks.mishinfz@...il.com>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH] iconv: Add check null-check for scd pointer

On Sun, Mar 24, 2024 at 10:25:03PM +0300, Maks Mishin wrote:
> After having been assigned to a NULL value at iconv.c:230,
> pointer 'scd' is dereferenced at iconv.c:383.
> 
> Found by RASU JSC.
> 
> Signed-off-by: Maks Mishin <maks.mishinFZ@...il.com>
> ---
>  src/locale/iconv.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/locale/iconv.c b/src/locale/iconv.c
> index 7fb2e1ef..e0d200b8 100644
> --- a/src/locale/iconv.c
> +++ b/src/locale/iconv.c
> @@ -232,6 +232,8 @@ size_t iconv(iconv_t cd, char **restrict in, size_t *restrict inb, char **restri
>  		scd = (void *)cd;
>  		cd = scd->base_cd;
>  	}
> +	if (scd == NULL) return x;
> +
>  	unsigned to = extract_to(cd);
>  	unsigned from = extract_from(cd);
>  	const unsigned char *map = charmaps+from+1;
> -- 
> 2.30.2

This makes iconv non-functional for non-stateful conversions. The
claim by the static analysis tool is false. It is not dereferenced in
the code path where it's null because in that code path,
type==ISO2022_JP is never true.

This tool you are using is really junk. You should stop sending
untested and obviously incorrect patches to projects, and advise any
projects that have accepted your patches that they may have been
dangerously incorrect.

Rich

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.