Обсуждение: BUG #15104: Double free in the main function in ecpg.c

The following bug has been logged on the website:

Bug reference:      15104
Logged by:          Pan Bian
Email address:      bianpan2016@163.com
PostgreSQL version: 10.3
Operating system:   Linux
Description:

File: src/interfaces/ecpg/preproc/ecpg.c
Function: main()

Details: In function main(), the memory hold by variable output_filename is
freed at line 329. It then tries to parse the next command option. The freed
memory will be freed again at line 478 if a crafted option bypass the memory
allocation at line 316. I think set output_filename to NULL after the free
operation at line 329 will fix the issue.

For your convenience, I paste related bugs as follows:

116 main(int argc, char *const argv[])
117 {
        ...
157     output_filename = NULL;

265     if (optind >= argc)         /* no files specified */
266     {
267         fprintf(stderr, _("%s: no input files specified\n"),
progname);
268         fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
argv[0]);
269         return (ILLEGAL_OPTION);
270     }
271     else
272     {
273         /* after the options there must not be anything but filenames
*/
274         for (fnr = optind; fnr < argc; fnr++)
275         {
                ...
310             if (out_option == 0)    /* calculate the output name */
311             {
312                 if (strcmp(input_filename, "stdin") == 0)
313                     base_yyout = stdout;
314                 else
315                 {
316                     output_filename = mm_alloc(strlen(input_filename) +
3);
317                     strcpy(output_filename, input_filename);
318 
319                     ptr2ext = strrchr(output_filename, '.');
320                     /* make extension = .c resp. .h */
321                     ptr2ext[1] = (header_mode == true) ? 'h' : 'c';
322                     ptr2ext[2] = '\0';
323 
324                     base_yyout = fopen(output_filename, PG_BINARY_W);
325                     if (base_yyout == NULL)
326                     {
327                         fprintf(stderr, _("%s: could not open file
\"%s\": %s\n"),
328                                 progname, output_filename,
strerror(errno));
329                         free(output_filename);
330                         free(input_filename);
331                         continue;
332                     }
333                 }
334             }
                ...
477             if (output_filename && out_option == 0)
478                 free(output_filename);
479 
480             free(input_filename);
481         }
482     }
483     return ret_value;
484 }

Thanks!

> Details: In function main(), the memory hold by variable
> output_filename is
> freed at line 329. It then tries to parse the next command option.
> The freed
> memory will be freed again at line 478 if a crafted option bypass the
> memory
> allocation at line 316. I think set output_filename to NULL after the
> free
> operation at line 329 will fix the issue.
> ...

I haven't had time yet to look into this, but to speed things up, would
you have an example where the double free actually happens?

Thanks.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL

On Fri, Mar 9, 2018 at 8:33 AM, Michael Meskes <meskes@postgresql.org> wrote:
>> Details: In function main(), the memory hold by variable
>> output_filename is
>> freed at line 329. It then tries to parse the next command option.
>> The freed
>> memory will be freed again at line 478 if a crafted option bypass the
>> memory
>> allocation at line 316. I think set output_filename to NULL after the
>> free
>> operation at line 329 will fix the issue.
>> ...
>
> I haven't had time yet to look into this, but to speed things up, would
> you have an example where the double free actually happens?
>
> Thanks.
>
> Michael
> --
> Michael Meskes
> Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
> Meskes at (Debian|Postgresql) dot Org
> Jabber: michael at xmpp dot meskes dot org
> VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
>

You can see it fairly easily by doing the following:

touch test.c;
chmod 0444 test.c;
echo "" | ./ecpg test.c -;

On my Mac I occasionally see the following:

ecpg(19715,0x7fffdd46b3c0) malloc: *** error for object
0x7fa92d402ed0: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
Abort trap: 6

I don't really understand how the double-free detection works, though,
and the error does not always appear.

There is a separate problem, when test.c *is* writeable and is
followed by an - (stdout) argument. In this case there is another
double free. Attached is a fix for both.

> There is a separate problem, when test.c *is* writeable and is
> followed by an - (stdout) argument. In this case there is another
> double free. Attached is a fix for both.

Thanks to both of you.

Committed.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL