diff mbox series

objdump.c: potential NULL pointer dereference in handle_ar

Message ID 20241023111414.2385429-1-ant.v.moryakov@gmail.com
State Dropped
Delegated to: Mark Wielaard
Headers
Series objdump.c: potential NULL pointer dereference in handle_ar |

Commit Message

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

- Added a check to ensure `arhdr` is not NULL before using it in `strcmp` to avoid segmentation fault.
- This resolves the issue where the pointer returned from `elf_getarhdr` may be NULL and causes a crash when dereferenced.
---
 src/objdump.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

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

On Wed, 2024-10-23 at 14:14 +0300, ant.v.moryakov@gmail.com wrote:
> From: AntonMoryakov <ant.v.moryakov@gmail.com>
> 
> - Added a check to ensure `arhdr` is not NULL before using it in `strcmp` to avoid segmentation fault.
> - This resolves the issue where the pointer returned from `elf_getarhdr` may be NULL and causes a crash when dereferenced.

Kind of the same questions/comments as for previous patches. Please
sign your work, keep with current coding style and if you have a
testcase that would be ideal.

> ---
>  src/objdump.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/objdump.c b/src/objdump.c
> index 1b38da23..33b6fec5 100644
> --- a/src/objdump.c
> +++ b/src/objdump.c
> @@ -313,7 +313,8 @@ handle_ar (int fd, Elf *elf, const char *prefix, const char *fname,
>        Elf_Arhdr *arhdr = elf_getarhdr (subelf);
>  
>        /* Skip over the index entries.  */
> -      if (strcmp (arhdr->ar_name, "/") != 0
> +      if (arhdr != NULL
> +    && strcmp (arhdr->ar_name, "/") != 0
>  	  && strcmp (arhdr->ar_name, "//") != 0)
>  	{
>  	  if (elf_kind (subelf) == ELF_K_ELF)

I think it would be better to do a separate check for arhdr being NULL,
that should normally not happen imho. Then do an INTERNAL_ERROR (fname)
to stop processing and report the libelf error. e.g something like:

diff --git a/src/objdump.c b/src/objdump.c
index 1b38da23266d..94cc69cb0f6a 100644
--- a/src/objdump.c
+++ b/src/objdump.c
@@ -311,6 +311,8 @@ handle_ar (int fd, Elf *elf, const char *prefix,
const char *fname,
     {
       /* The the header for this element.  */
       Elf_Arhdr *arhdr = elf_getarhdr (subelf);
+      if (ahdr == NULL)
+       INTERNAL_ERROR (fname);
 
       /* Skip over the index entries.  */
       if (strcmp (arhdr->ar_name, "/") != 0

Cheers,

Mark
diff mbox series

Patch

diff --git a/src/objdump.c b/src/objdump.c
index 1b38da23..33b6fec5 100644
--- a/src/objdump.c
+++ b/src/objdump.c
@@ -313,7 +313,8 @@  handle_ar (int fd, Elf *elf, const char *prefix, const char *fname,
       Elf_Arhdr *arhdr = elf_getarhdr (subelf);
 
       /* Skip over the index entries.  */
-      if (strcmp (arhdr->ar_name, "/") != 0
+      if (arhdr != NULL
+    && strcmp (arhdr->ar_name, "/") != 0
 	  && strcmp (arhdr->ar_name, "//") != 0)
 	{
 	  if (elf_kind (subelf) == ELF_K_ELF)