Re: [PATCH] Fix several buffer overruns in gcov

[Bernd Edlinger]

On 03/31/17 01:27, Nathan Sidwell wrote:
> On 03/30/2017 04:11 PM, Bernd Edlinger wrote:
>> Hi,
>>
>> I'd like to fix a few buffer overruns I have found in the gcov tools.
>> First I noticed that the -x output contains most of the time "ff" bytes,
>> and that when different source files exist in different directories,
>> with same base name the MD5 sum always matches, which results in
>> gcov overwriting the previous result file always, except if -l is given,
>> which makes hashing the file names practically useless.
>>
>> And secondly I wanted to fix potential buffer underflow if a file
>> contains lines with begin with NUL ascii characters, and a out of
>> memory due to always doubling the buffer space, even if the line
>> buffer is not yet filled up.
>>
>>
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
>
> ok.  Could you put a comment on the buffer reallocation test about NUL
> defense, thanks!
>

Thanks for the quick response!
I added a comment and commited as r246605:

Index: gcc/gcov.c
===================================================================
--- gcc/gcov.c  (revision 246604)
+++ gcc/gcov.c  (revision 246605)
@@ -2167,7 +2167,7 @@
  md5sum_to_hex (const char *sum, char *buffer)
  {
    for (unsigned i = 0; i < 16; i++)
-    sprintf (buffer + (2 * i), "%02x", sum[i]);
+    sprintf (buffer + (2 * i), "%02x", (unsigned char)sum[i]);
  }

  /* Generate an output file name. INPUT_NAME is the canonicalized main
@@ -2216,7 +2216,7 @@
        char md5sum_hex[33];

        md5_init_ctx (&ctx);
-      md5_process_bytes (result, strlen (result), &ctx);
+      md5_process_bytes (src_name, strlen (src_name), &ctx);
        md5_finish_ctx (&ctx, md5sum);
        md5sum_to_hex (md5sum, md5sum_hex);
        free (result);
@@ -2512,14 +2512,20 @@
      {
        size_t len = strlen (string + pos);

-      if (string[pos + len - 1] == '\n')
+      if (len && string[pos + len - 1] == '\n')
        {
          string[pos + len - 1] = 0;
          return string;
        }
        pos += len;
-      string = XRESIZEVEC (char, string, string_len * 2);
-      string_len *= 2;
+      /* If the file contains NUL characters or an incomplete
+        last line, which can happen more than once in one run,
+        we have to avoid doubling the STRING_LEN unnecessarily.  */
+      if (pos > string_len / 2)
+       {
+         string_len *= 2;
+         string = XRESIZEVEC (char, string, string_len);
+       }
      }

    return pos ? string : NULL;
Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog       (revision 246604)
+++ gcc/ChangeLog       (revision 246605)
@@ -1,3 +1,10 @@
+2017-03-31  Bernd Edlinger  <bernd.edlin...@hotmail.de>
+
+       * gcov.c (md5sum_to_hex): Fix output of MD5 hex bytes.
+       (make_gcov_file_name): Use the canonical path name for generating
+       the MD5 value.
+       (read_line): Fix handling of files with ascii null bytes.
+
  2017-03-30  Matthew Fortune  <matthew.fort...@imgtec.com>

        * config/mips/mips.c (mips_expand_vector_init): Create a const_vector


PS: Could you also please have a look at
https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01434.html


Thanks
Bernd.