[Bug lto/41550] New: Fix security and portability issues in lto-plugin

[Bug lto/41550] New: Fix security and portability issues in lto-plugin

[dnovillo at gcc dot gnu dot org]

>From http://gcc.gnu.org/ml/gcc-patches/2009-09/msg02151.html

> +static void
> +write_resolution (void)
> +{
> +  unsigned int i;
> +  FILE *f;
> +  /* FIXME: Disabled for now since we are not using the resolution file. */
> +  return;
> +
> +
> +  /* FIXME: This should be a temporary file. */
> +  f = fopen ("resolution", "w");

Fixed filenames like this are typically security holes.  The return above 
means this isn't, but it might be best to remove this function altogether 
until you are ready to add a version that actually does something.

> +/* Pass files generated by the lto-wrapper to the linker. FD is lto-wrapper's
> +   stdout. */
> +
> +static void
> +add_output_files (FILE *f)
> +{
> +  char fname[1000]; /* FIXME: Is this big enough? */

I don't know what sort of strings go there, but if they can be filenames 
with user-controlled components then the GNU Coding Standards say to avoid 
arbitrary limits.

> +      output_files = realloc (output_files, num_output_files * sizeof (char *));
> +      output_files[num_output_files - 1] = strdup (s);

Use xrealloc and xstrdup.  Other places have the same issue with realloc 
or calloc or strdup.

> +  /* Write argv to a file to avoid a command line that is too long. */
> +  t = asprintf (&at_args, "@%s/arguments", temp_obj_dir_name);
> +  assert (t >= 0);

This is an example of inappropriate use of assert for things that may be 
valid error conditions not a program bug.  It looks like there are others 
in this plugin.  Some cleanup work is needed.


-- 
           Summary: Fix security and portability issues in lto-plugin
           Product: gcc
           Version: lto
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: lto
        AssignedTo: espindola at google dot com
        ReportedBy: dnovillo at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41550

[Bug lto/41550] Fix security and portability issues in lto-plugin

[rguenth at gcc dot gnu dot org]

------- Comment #1 from rguenth at gcc dot gnu dot org  2009-10-03 22:16 -------
See also PR39023.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |build


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41550

[Bug lto/41550] Fix security and portability issues in lto-plugin

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2009-10-31 13:11 -------
Some things were fixed.  Still open are

> +/* Pass files generated by the lto-wrapper to the linker. FD is lto-wrapper's
> +   stdout. */
> +
> +static void
> +add_output_files (FILE *f)
> +{
> +  char fname[1000]; /* FIXME: Is this big enough? */

I don't know what sort of strings go there, but if they can be filenames 
with user-controlled components then the GNU Coding Standards say to avoid 
arbitrary limits.

> +      output_files = realloc (output_files, num_output_files * sizeof (char *));
> +      output_files[num_output_files - 1] = strdup (s);

Use xrealloc and xstrdup.  Other places have the same issue with realloc 
or calloc or strdup.


Also there are still asserts that look fishy.

  assert (lto_wrapper_argv);

  temp_obj_dir_name = strdup ("tmp_objectsXXXXXX");
  t = mkdtemp (temp_obj_dir_name);
  assert (t == temp_obj_dir_name);

(see also PR39023)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41550

[Bug lto/41550] Fix security and portability issues in lto-plugin

[rguenth at gcc dot gnu dot org]

------- Comment #3 from rguenth at gcc dot gnu dot org  2010-04-23 15:05 -------
I have patches.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|espindola at google dot com |rguenth at gcc dot gnu dot
                   |                            |org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2010-04-23 15:05:28
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41550

[Bug lto/41550] Fix security and portability issues in lto-plugin

[rguenth at gcc dot gnu dot org]

------- Comment #4 from rguenth at gcc dot gnu dot org  2010-04-23 15:49 -------
Fixed.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |4.5.0
   Target Milestone|---                         |4.6.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41550

[Bug lto/41550] Fix security and portability issues in lto-plugin

[rguenth at gcc dot gnu dot org]

------- Comment #6 from rguenth at gcc dot gnu dot org  2010-04-23 15:49 -------
,


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41550

[Bug lto/41550] Fix security and portability issues in lto-plugin

[rguenth at gcc dot gnu dot org]

------- Comment #5 from rguenth at gcc dot gnu dot org  2010-04-23 15:49 -------
Subject: Bug 41550

Author: rguenth
Date: Fri Apr 23 15:49:10 2010
New Revision: 158673

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=158673
Log:
2010-04-23  Richard Guenther  <rguenther@suse.de>

        PR lto/41550
        * lto-plugin.c (parse_table_entry): Use xstrdup and xrealloc.
        (translate): Likewise.
        (all_symbols_read_handler): Likewise.
        (claim_file_handler): Likewise.
        (process_option): Likewise.
        (add_output_files): Likewise.  Remove filename length limit.

Modified:
    trunk/lto-plugin/ChangeLog
    trunk/lto-plugin/lto-plugin.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41550