diff mbox series

size: Fix deref-of-null in handle_ar() function

Message ID 20240701201556.9122-1-maks.mishinFZ@gmail.com
State Dropped
Headers
Series size: Fix deref-of-null in handle_ar() function |

Commit Message

Maks Mishin July 1, 2024, 8:15 p.m. UTC 
  Pointer, returned from function 'elf_getarhdr' at size.c:362,
may be NULL and is dereferenced at size.c:367.

Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com>
---
 src/size.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Sam James July 1, 2024, 8:54 p.m. UTC | #1 
Maks Mishin <maks.mishinfz@gmail.com> writes:

> Pointer, returned from function 'elf_getarhdr' at size.c:362,
> may be NULL and is dereferenced at size.c:367.
>

Your other patch has "Found by RASU JSC." but the rest don't. Are they
all found by it? If so, please say that.

(Also, consider sending the fixes as a series if they're related/from
the same analyser tool batch.)

> Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com>
> ---
>  src/size.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/src/size.c b/src/size.c
> index ff8ca075..d6bce203 100644
> --- a/src/size.c
> +++ b/src/size.c
> @@ -361,6 +361,16 @@ 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 (arhdr == NULL)
> +  {
> +    printf ("cannot get archive header in '%s': %s\n",
> +      fname, elf_errmsg (-1));
> +    elf_end (subelf);
> +    elf_end (elf);
> +    close (fd);
> +    return 1;
> +  }
> +
>        if (elf_kind (subelf) == ELF_K_ELF)
>  	handle_elf (subelf, new_prefix, arhdr->ar_name);
>        else if (likely (elf_kind (subelf) == ELF_K_AR))
Mark Wielaard July 2, 2024, 11:21 a.m. UTC | #2 
Hi,

On Mon, Jul 01, 2024 at 09:54:46PM +0100, Sam James wrote:
> Maks Mishin <maks.mishinfz@gmail.com> writes:
> 
> > Pointer, returned from function 'elf_getarhdr' at size.c:362,
> > may be NULL and is dereferenced at size.c:367.
> >
> 
> Your other patch has "Found by RASU JSC." but the rest don't. Are they
> all found by it? If so, please say that.

Also please explain why a patch is needed.  Last time you submitted
patches I just gave up reviewing them because most of them were just
bogus. Using a tool to find issues is fine, but you do need to explain
why you believe the tool reports are correct.

https://inbox.sourceware.org/elfutils-devel/20240405190452.GH1292@gnu.wildebeest.org/

Cheers,

Mark
diff mbox series

Patch

diff --git a/src/size.c b/src/size.c
index ff8ca075..d6bce203 100644
--- a/src/size.c
+++ b/src/size.c
@@ -361,6 +361,16 @@  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 (arhdr == NULL)
+  {
+    printf ("cannot get archive header in '%s': %s\n",
+      fname, elf_errmsg (-1));
+    elf_end (subelf);
+    elf_end (elf);
+    close (fd);
+    return 1;
+  }
+
       if (elf_kind (subelf) == ELF_K_ELF)
 	handle_elf (subelf, new_prefix, arhdr->ar_name);
       else if (likely (elf_kind (subelf) == ELF_K_AR))