diff mbox series

iconv: skeleton.c: Fix potential NULL dereference in FUNCTION_NAME

Message ID 20251029141606.59951-2-ant.v.moryakov@gmail.com (mailing list archive)
State Changes Requested
Headers
Series iconv: skeleton.c: Fix potential NULL dereference in FUNCTION_NAME |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686

Commit Message

Anton Moryakov Oct. 29, 2025, 2:16 p.m. UTC 
  Report of the static analyzer:
After being compared to a NULL value at skeleton.c:516, pointer
'irreversible' is dereferenced at skeleton.c:662. This indicates a
potential null pointer dereference vulnerability.

Correct explained:
The pointer 'irreversible' is checked for NULL when initializing
'lirreversiblep' (used in conversion loops), but later unconditionally
dereferenced in the exit path when updating the irreversible counter.
This creates an inconsistency: if the function is called with
irreversible == NULL, and the conversion loop completes successfully,
the final update '*irreversible += lirreversible' will cause a
segmentation fault.

Add a NULL check before dereferencing to prevent the crash. This ensures
consistent behavior with the earlier initialization logic and eliminates
the risk of undefined behavior.

Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
---
 iconv/skeleton.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Adhemerval Zanella Netto Oct. 29, 2025, 4:59 p.m. UTC | #1 
On 29/10/25 11:16, Anton Moryakov wrote:
> Report of the static analyzer:
> After being compared to a NULL value at skeleton.c:516, pointer
> 'irreversible' is dereferenced at skeleton.c:662. This indicates a
> potential null pointer dereference vulnerability.
> 
> Correct explained:
> The pointer 'irreversible' is checked for NULL when initializing
> 'lirreversiblep' (used in conversion loops), but later unconditionally
> dereferenced in the exit path when updating the irreversible counter.
> This creates an inconsistency: if the function is called with
> irreversible == NULL, and the conversion loop completes successfully,
> the final update '*irreversible += lirreversible' will cause a
> segmentation fault.
> 
> Add a NULL check before dereferencing to prevent the crash. This ensures
> consistent behavior with the earlier initialization logic and eliminates
> the risk of undefined behavior.
> 
> Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>

Do we have a testcase that triggers this issue? And do we need a bug report
for this?

> ---
>  iconv/skeleton.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/iconv/skeleton.c b/iconv/skeleton.c
> index 7523694b81..4565beff33 100644
> --- a/iconv/skeleton.c
> +++ b/iconv/skeleton.c
> @@ -533,7 +533,8 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
>  
>  	      /* Remember how many non-identical characters we
>  		 converted in an irreversible way.  */
> -	      *irreversible += lirreversible;
> +	      if (irreversible != NULL)
> +	      	*irreversible += lirreversible;
>  
>  	      break;
>  	    }
diff mbox series

Patch

diff --git a/iconv/skeleton.c b/iconv/skeleton.c
index 7523694b81..4565beff33 100644
--- a/iconv/skeleton.c
+++ b/iconv/skeleton.c
@@ -533,7 +533,8 @@  FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
 
 	      /* Remember how many non-identical characters we
 		 converted in an irreversible way.  */
-	      *irreversible += lirreversible;
+	      if (irreversible != NULL)
+	      	*irreversible += lirreversible;
 
 	      break;
 	    }