[PATCH] [iconv] Fix possible null-pointer dereference.

[PATCH] [iconv] Fix possible null-pointer dereference.

[Dmitry Chestnyh]

This issue was found by SVACE static analyzer.
Dereference can appear at line 665 and there are no
obvious checks of `irrecersible` ptr value.
And seems that we can't be sure that this pointer
isn't NULL.
---
 iconv/skeleton.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/iconv/skeleton.c b/iconv/skeleton.c
index 0356dbf92b..a296a9f944 100644
--- a/iconv/skeleton.c
+++ b/iconv/skeleton.c
@@ -659,6 +659,7 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
 	      /* Store information about how many bytes are available.  */
 	      data->__outbuf = outbuf;
 
+	      assert(irreversible != NULL);
 	      /* Remember how many non-identical characters we
 		 converted in an irreversible way.  */
 	      *irreversible += lirreversible;
-- 
2.25.1

Re: [PATCH] [iconv] Fix possible null-pointer dereference.

[Andreas Schwab]

On Mär 31 2022, Dmitry Chestnyh wrote:

> diff --git a/iconv/skeleton.c b/iconv/skeleton.c
> index 0356dbf92b..a296a9f944 100644
> --- a/iconv/skeleton.c
> +++ b/iconv/skeleton.c
> @@ -659,6 +659,7 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
>  	      /* Store information about how many bytes are available.  */
>  	      data->__outbuf = outbuf;
>  
> +	      assert(irreversible != NULL);
>  	      /* Remember how many non-identical characters we
>  		 converted in an irreversible way.  */
>  	      *irreversible += lirreversible;

I don't think this function is ever called with irreversible == NULL.
In fact, there is such an assertion in __gconv.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] [iconv] Fix possible null-pointer dereference.

[Andreas Schwab]

On Mär 31 2022, Andreas Schwab wrote:

> On Mär 31 2022, Dmitry Chestnyh wrote:
>
>> diff --git a/iconv/skeleton.c b/iconv/skeleton.c
>> index 0356dbf92b..a296a9f944 100644
>> --- a/iconv/skeleton.c
>> +++ b/iconv/skeleton.c
>> @@ -659,6 +659,7 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
>>  	      /* Store information about how many bytes are available.  */
>>  	      data->__outbuf = outbuf;
>>  
>> +	      assert(irreversible != NULL);
>>  	      /* Remember how many non-identical characters we
>>  		 converted in an irreversible way.  */
>>  	      *irreversible += lirreversible;
>
> I don't think this function is ever called with irreversible == NULL.
> In fact, there is such an assertion in __gconv.

Except that __gconv_transliterate does pass NULL here.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] [iconv] Fix possible null-pointer dereference.

[Dmitriy Chestnyh]

So the patch may be useful?

________________________________
От: Andreas Schwab <schwab@linux-m68k.org>
Отправлено: 31 марта 2022 г. 18:56:08
Кому: Dmitriy Chestnyh
Копия: libc-alpha@sourceware.org
Тема: Re: [PATCH] [iconv] Fix possible null-pointer dereference.

On Mär 31 2022, Andreas Schwab wrote:

> On Mär 31 2022, Dmitry Chestnyh wrote:
>
>> diff --git a/iconv/skeleton.c b/iconv/skeleton.c
>> index 0356dbf92b..a296a9f944 100644
>> --- a/iconv/skeleton.c
>> +++ b/iconv/skeleton.c
>> @@ -659,6 +659,7 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
>>             /* Store information about how many bytes are available.  */
>>             data->__outbuf = outbuf;
>>
>> +          assert(irreversible != NULL);
>>             /* Remember how many non-identical characters we
>>                converted in an irreversible way.  */
>>             *irreversible += lirreversible;
>
> I don't think this function is ever called with irreversible == NULL.
> In fact, there is such an assertion in __gconv.

Except that __gconv_transliterate does pass NULL here.

--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] [iconv] Fix possible null-pointer dereference.

[Andreas Schwab]

It may actually be possible to reach this point with irreversible ==
NULL.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] iconv: Fix possible null-pointer dereference.

[Andreas Schwab]

On Mär 31 2022, Dmitry Chestnyh wrote:

> The dereference can appear at iconv.c:54.

When inbuf isn't NULL then outbuf cannot be NULL.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

[PATCH] iconv: Fix possible null-pointer dereference.

[Dmitry Chestnyh]

The dereference can appear at iconv.c:54.
This problem was found by SVACE static analyzer.
---
 iconv/iconv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/iconv/iconv.c b/iconv/iconv.c
index e805c4c59d..1c3c517ad3 100644
--- a/iconv/iconv.c
+++ b/iconv/iconv.c
@@ -35,7 +35,7 @@ iconv (iconv_t cd, char **inbuf, size_t *inbytesleft, char **outbuf,
   size_t irreversible;
   int result;
 
-  if (__glibc_unlikely (inbuf == NULL || *inbuf == NULL))
+  if (__glibc_unlikely (inbuf == NULL || *inbuf == NULL || outbuf == NULL))
     {
       if (outbuf == NULL || *outbuf == NULL)
 	result = __gconv (gcd, NULL, NULL, NULL, NULL, &irreversible);
-- 
2.25.1