dwfl_segment_report_module.c: Fix potential NULL pointer dereference in handle_file_note

Message ID 20241023110809.2385085-1-ant.v.moryakov@gmail.com
State Dropped
Delegated to: Mark Wielaard
Headers
Series dwfl_segment_report_module.c: Fix potential NULL pointer dereference in handle_file_note |

Commit Message

Anton Moryakov Oct. 23, 2024, 11:08 a.m. UTC
  From: AntonMoryakov <ant.v.moryakov@gmail.com>

- Added a check to ensure `retval` is not NULL before using it in `strcmp` to prevent a segmentation fault.
- This resolves the issue where `retval` could be NULL when passed to `strcmp`, which could cause a crash.
---
 libdwfl/dwfl_segment_report_module.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Mark Wielaard Oct. 23, 2024, 1:03 p.m. UTC | #1
Hi Anton,

On Wed, 2024-10-23 at 14:08 +0300, ant.v.moryakov@gmail.com wrote:
> From: AntonMoryakov <ant.v.moryakov@gmail.com>
> 
> - Added a check to ensure `retval` is not NULL before using it in `strcmp` to prevent a segmentation fault.
> - This resolves the issue where `retval` could be NULL when passed to `strcmp`, which could cause a crash.

Are you sure? A testcase would be nice.

> ---
>  libdwfl/dwfl_segment_report_module.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libdwfl/dwfl_segment_report_module.c b/libdwfl/dwfl_segment_report_module.c
> index 32f44af8..d2512cb3 100644
> --- a/libdwfl/dwfl_segment_report_module.c
> +++ b/libdwfl/dwfl_segment_report_module.c
> @@ -205,7 +205,7 @@ handle_file_note (GElf_Addr module_start, GElf_Addr module_end,
>  	return NULL;
>        if (mix == firstix)
>  	retval = fptr;
> -      if (firstix < mix && mix <= lastix && strcmp (fptr, retval) != 0)
> +      if (retval != NULL && firstix < mix && mix <= lastix && strcmp (fptr, retval) != 0)
>  	return NULL;
>        fptr = fnext + 1;
>      }

This came up before so maybe a comment could be added.
https://inbox.sourceware.org/elfutils-devel/20240702111528.GA29242@gnu.wildebeest.org/
How did you determine this patch was necessary?

It seems that retval is definitely set in this case.

Cheers,

Mark
  

Patch

diff --git a/libdwfl/dwfl_segment_report_module.c b/libdwfl/dwfl_segment_report_module.c
index 32f44af8..d2512cb3 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -205,7 +205,7 @@  handle_file_note (GElf_Addr module_start, GElf_Addr module_end,
 	return NULL;
       if (mix == firstix)
 	retval = fptr;
-      if (firstix < mix && mix <= lastix && strcmp (fptr, retval) != 0)
+      if (retval != NULL && firstix < mix && mix <= lastix && strcmp (fptr, retval) != 0)
 	return NULL;
       fptr = fnext + 1;
     }