Re: [Tinycc-devel] NULL pointer dereference due to unchecked return from fdopen()

[Christian Jullien]

Hi,

I'm really sorry to hurt you. It looks John had an issue because of unchecked 
returned value, I was just trying to help.
IMHO, if fdopen really fails, as first approach, it's better to have an error 
message than a core dump but you're the maintainer and I respect all your 
decisions, you have probably something better to propose when fdopen fails.
I suppose that, from John point of view, the problem is not non-existent.

M2c.

C. 

-----Original Message-----
From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=orange.fr@nongnu.org] On 
Behalf Of grischka
Sent: Monday, February 28, 2022 10:50
To: jullien@eligis.com; tinycc-devel@nongnu.org
Subject: Re: [Tinycc-devel] NULL pointer dereference due to unchecked return 
from fdopen()

Christian Jullien wrote:
> Thanks,
> This is unfortunately not the only case where returned value is not tested, 
> just for fdopen, if maintainers agree, we can probably apply:
> Wdyt?

The rule is, as always:  don't write code that you cannot test.

Can you?

Otherwise, can we stop suggesting sloppily crafted quick patches
addressing non-existent problems?

Is that possible, then?

-- gr

> git diff tcc*.c
> diff --git a/tccelf.c b/tccelf.c
> index 507e83c..bd0a1d9 100644
> --- a/tccelf.c
> +++ b/tccelf.c
> @@ -2428,6 +2428,9 @@ static int tcc_write_elf_file(TCCState *s1, const char 
> *filename, int phnum,
>          return -1;
>      }
>      f = fdopen(fd, "wb");
> +    if (f == NULL) {
> +        tcc_error("Unable to fdopen %s for output", filename);
> +    }
>      if (s1->verbose)
>          printf("<- %s\n", filename);
>
> diff --git a/tccmacho.c b/tccmacho.c
> index 57c62c3..f94f976 100644
> --- a/tccmacho.c
> +++ b/tccmacho.c
> @@ -800,6 +800,9 @@ ST_FUNC int macho_output_file(TCCState *s1, const char 
> *filename)
>          return -1;
>      }
>      fp = fdopen(fd, "wb");
> +    if (fp == NULL) {
> +        tcc_error("Unable to fdopen %s for output", filename);
> +    }
>      if (s1->verbose)
>          printf("<- %s\n", filename);
>
>
>
>
> -----Original Message-----
> From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=orange.fr@nongnu.org] 
> On Behalf Of John Scott
> Sent: Monday, February 28, 2022 05:18
> To: tinycc-devel@nongnu.org
> Subject: [Tinycc-devel] NULL pointer dereference due to unchecked return from 
> fdopen()
>
> Hi all,
>
> I found this bug using the oomify tool at https://github.com/tavianator/oomify
>
> The problem can be seen at tccelf.c around line 2430 (f has type FILE*):
>       f = fdopen(fd, "wb");
>       if (s1->verbose)
>               printf("<- %s\n", filename);
>
> #ifdef TCC_TARGET_COFF
>       if (s1->output_format == TCC_OUTPUT_FORMAT_COFF)
>               tcc_output_coff(s1, f);
>       else
> #endif
>       if (s1->output_format == TCC_OUTPUT_FORMAT_ELF)
>               tcc_output_elf(s1, f, phnum, phdr, file_offset, sec_order);
>
> Note that the return value from fdopen() is not checked if it is NULL.
> If the output format is ELF, then tcc_output_elf() expects that f is a valid 
> FILE* variable and passes it to fwrite(), which causes undefined behavior.
>
> I don't know how to fix this, but hope that maybe one of you folks will 
> appreciate this report.
>
>
> _______________________________________________
> Tinycc-devel mailing list
> Tinycc-devel@nongnu.org
> https://lists.nongnu.org/mailman/listinfo/tinycc-devel


_______________________________________________
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel