diff mbox series

[iconv] Fix possible null-pointer dereference.

Message ID 20220331151918.433882-1-d.chestnyh@omp.ru
State New
Headers
Series [iconv] Fix possible null-pointer dereference. |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Dmitry Chestnyh March 31, 2022, 3:19 p.m. UTC 
  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(+)

Comments

Andreas Schwab March 31, 2022, 3:47 p.m. UTC | #1 
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 March 31, 2022, 3:56 p.m. UTC | #2 
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.
Dmitry Chestnyh March 31, 2022, 3:58 p.m. UTC | #3 
So the patch may be useful?
Andreas Schwab March 31, 2022, 4:43 p.m. UTC | #4 
It may actually be possible to reach this point with irreversible ==
NULL.
diff mbox series

Patch

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;