Subject: bug#47362: closed (Re: [bug-diffutils] bug#47362: important potential issues found by covscan in diffutils-3.7 on fedora)

From: [email-masked] (GNU bug Tracking System)
Date: Wed, 24 Mar 2021 21:17:02 +0000

Your bug report

#47362: important potential issues found by covscan in diffutils-3.7 on fedora

which was filed against the diffutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
47362: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=47362
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: [bug-diffutils] bug#47362: important potential issues found by covscan in diffutils-3.7 on fedora
From: Paul Eggert <[email-masked]>
To: than <[email-masked]>
Cc: [email-masked]
Date: Wed, 24 Mar 2021 14:16:26 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Thanks for the bug report. Those are all false alarms or are already 
fixed in Gnulib, except for a memory leak in ifdef.c for which I 
installed the attached patch. Thanks for reporting the problem.
From 1399b225ebecdbac80cc14be87c2454311f3b40f Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Wed, 24 Mar 2021 14:12:22 -0700
Subject: [PATCH] diff: plug memory leak in ifdef.c
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem reported by Than (Bug#47362).
Fix the bug by using xmalloca instead of vararrays.
* bootstrap.conf (gnulib_modules): Add xmalloca; remove vararrays.
* configure.ac: Remove AC_C_VARARRAYS.
* src/ifdef.c: Include xmalloca.h instead of xalloc.h.
(do_printf_spec): Use xmalloca instead of an xmalloc
that lacks a corresponding ‘free’ if HAVE_C_VARARRAYS
due to a typo in 2017-05-18T05:51:[email-masked].
---
 bootstrap.conf |  2 +-
 configure.ac   |  1 -
 src/ifdef.c    | 12 +++---------
 3 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index d45abdb..f2359f6 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -81,13 +81,13 @@ system-quote
 unistd
 unlocked-io
 update-copyright
-vararrays
 verify
 version-etc
 version-etc-fsf
 wcwidth
 xalloc
 xfreopen
+xmalloca
 xreadlink
 xstdopen
 xstrtoimax
diff --git a/configure.ac b/configure.ac
index 3b2195e..6a76b12 100644
--- a/configure.ac
+++ b/configure.ac
@@ -111,7 +111,6 @@ if test "$gl_gcc_warnings" = yes; then
 fi
 
 AC_C_INLINE
-AC_C_VARARRAYS
 
 AC_DEFINE([DEFAULT_EDITOR_PROGRAM], ["ed"],
   [Name of editor program, unless overridden.])
diff --git a/src/ifdef.c b/src/ifdef.c
index 8e64b4c..d40a88e 100644
--- a/src/ifdef.c
+++ b/src/ifdef.c
@@ -22,7 +22,7 @@
 
 #include "diff.h"
 
-#include <xalloc.h>
+#include <xmalloca.h>
 
 struct group
 {
@@ -362,20 +362,14 @@ do_printf_spec (FILE *out, char const *spec,
             printint print_value = value;
             size_t spec_prefix_len = f - spec - 2;
             size_t pI_len = sizeof pI - 1;
-#if 0
-            char format[spec_prefix_len + pI_len + 2];
-#else
-            char *format = xmalloc (spec_prefix_len + pI_len + 2);
-#endif
+            char *format = xmalloca (spec_prefix_len + pI_len + 2);
             char *p = format + spec_prefix_len + pI_len;
             memcpy (format, spec, spec_prefix_len);
             memcpy (format + spec_prefix_len, pI, pI_len);
             *p++ = c;
             *p = '\0';
             fprintf (out, format, print_value);
-#if ! HAVE_C_VARARRAYS
-            free (format);
-#endif
+            freea (format);
           }
       }
       break;
-- 
2.27.0

Subject: important potential issues found by covscan in diffutils-3.7 on fedora
From: than <[email-masked]>
To: [email-masked]
Date: Wed, 24 Mar 2021 15:07:15 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Dear diffutil devs,

Covscan found 10important potentialin diffutils-3.7 on fedora. The 
Coverity covscan result is attached below. It could be that some of them 
are false positive but it's worth checking the coverity covscan result.

Thanks!

Best Regards,

Than

List of Defects:
Error: COMPILER_WARNING (CWE-758): [#def1]
diffutils-3.7/lib/exclude.c:636:18: warning[-Wstringop-overflow=]: 
writing 1 byte into a region of size 0
#  636 |   buf[buf_count] = line_end;
#      |                  ^
#  634|
#  635|     buf = xrealloc (buf, buf_count + 1);
#  636|->   buf[buf_count] = line_end;
#  637|     lim = buf + buf_count + ! (buf_count == 0 || buf[buf_count - 
1] == line_end);
#  638|

Error: RESOURCE_LEAK (CWE-772): [#def2]
diffutils-3.7/lib/freopen-safer.c:42: open_fn: Returning handle opened 
by "open". [Note: The source code implementation of the function has 
been overridden by a user model.]
diffutils-3.7/lib/freopen-safer.c:42: var_assign: Assigning: "value" = 
handle returned from "open("/dev/null", 0)".
diffutils-3.7/lib/freopen-safer.c:52: leaked_handle: Handle variable 
"value" going out of scope leaks the handle.
#   50|         return false;
#   51|       }
#   52|->   return true;
#   53|   }
#   54|

Error: CPPCHECK_WARNING (CWE-401): [#def3]
diffutils-3.7/lib/malloca.c:67: error[memleak]: Memory leak: mem
#   65|             ((small_t *) p)[-1] = p - mem;
#   66|             /* p  sa_alignment_max mod 2*sa_alignment_max.  */
#   67|->           return p;
#   68|           }
#   69|       }

Error: RESOURCE_LEAK (CWE-772): [#def4]
diffutils-3.7/lib/regex_internal.c:1684: alloc_fn: Storage is returned 
from allocation function "calloc".
diffutils-3.7/lib/regex_internal.c:1684: var_assign: Assigning: 
"newstate" = storage returned from "calloc(112UL, 1UL)".
diffutils-3.7/lib/regex_internal.c:1687: noescape: Resource 
"&newstate->nodes" is not freed or pointed-to in "re_node_set_init_copy".
diffutils-3.7/lib/regex_internal.c:1734: noescape: Resource 
"&newstate->nodes" is not freed or pointed-to in "re_node_set_remove_at".
diffutils-3.7/lib/regex_internal.c:1734: noescape: Resource 
"&newstate->nodes" is not freed or pointed-to in "re_node_set_remove_at".
diffutils-3.7/lib/regex_internal.c:1734: noescape: Resource 
"&newstate->nodes" is not freed or pointed-to in "re_node_set_remove_at".
diffutils-3.7/lib/regex_internal.c:1727: leaked_storage: Variable 
"newstate" going out of scope leaks the storage it points to.
# 1725|             if (re_node_set_init_copy (newstate->entrance_nodes, 
nodes)
# 1726|             != REG_NOERROR)
# 1727|->         return NULL;
# 1728|             nctx_nodes = 0;
# 1729|             newstate->has_constraint = 1;

Error: BAD_FREE (CWE-763): [#def5]
diffutils-3.7/src/analyze.c:691: offset_free: "free" frees address 
offset from "cmp->file[f].linbuf".
#  689|       {
#  690|         free (cmp->file[f].equivs);
#  691|->       free (cmp->file[f].linbuf + cmp->file[f].linbuf_base);
#  692|       }
#  693|

Error: RESOURCE_LEAK (CWE-772): [#def6]
diffutils-3.7/src/diff3.c:783: alloc_fn: Storage is returned from 
allocation function "create_diff3_block".
diffutils-3.7/src/diff3.c:783: var_assign: Assigning: "result" = storage 
returned from "create_diff3_block(low[0], high[0], low[1], high[1], 
lowc, highc)".
diffutils-3.7/src/diff3.c:798: leaked_storage: Variable "result" going 
out of scope leaks the storage it points to.
#  796|                     D_LENARRAY (result, FILEC) + result_offset,
#  797|                     D_NUMLINES (ptr, FC)))
#  798|->       return 0;
#  799|         }
#  800|

Error: RESOURCE_LEAK (CWE-772): [#def7]
diffutils-3.7/src/diff3.c:783: alloc_fn: Storage is returned from 
allocation function "create_diff3_block".
diffutils-3.7/src/diff3.c:783: var_assign: Assigning: "result" = storage 
returned from "create_diff3_block(low[0], high[0], low[1], high[1], 
lowc, highc)".
diffutils-3.7/src/diff3.c:827: leaked_storage: Variable "result" going 
out of scope leaks the storage it points to.
#  825|                   D_LENARRAY (result, FILE0 + d) + result_offset,
#  826|                   D_NUMLINES (ptr, FO)))
#  827|->         return 0;
#  828|
#  829|         /* Catch the lines between here and the next diff */

Error: RESOURCE_LEAK (CWE-772): [#def8]
diffutils-3.7/src/ifdef.c:368: alloc_fn: Storage is returned from 
allocation function "xmalloc".
diffutils-3.7/src/ifdef.c:368: var_assign: Assigning: "format" = storage 
returned from "xmalloc(spec_prefix_len + pI_len + 2UL)".
diffutils-3.7/src/ifdef.c:370: var_assign: Assigning: "p" = "format".
diffutils-3.7/src/ifdef.c:371: noescape: Resource "format" is not freed 
or pointed-to in "memcpy". [Note: The source code implementation of the 
function has been overridden by a builtin model.]
diffutils-3.7/src/ifdef.c:372: noescape: Resource "format + 
spec_prefix_len" is not freed or pointed-to in "memcpy". [Note: The 
source code implementation of the function has been overridden by a 
builtin model.]
diffutils-3.7/src/ifdef.c:375: noescape: Resource "format" is not freed 
or pointed-to in "fprintf". [Note: The source code implementation of the 
function has been overridden by a builtin model.]
diffutils-3.7/src/ifdef.c:379: leaked_storage: Variable "p" going out of 
scope leaks the storage it points to.
diffutils-3.7/src/ifdef.c:379: leaked_storage: Variable "format" going 
out of scope leaks the storage it points to.
#  377|           free (format);
#  378|   #endif
#  379|->       }
#  380|         }
#  381|         break;

Error: RESOURCE_LEAK (CWE-772): [#def9]
diffutils-3.7/src/sdiff.c:1166: alloc_fn: Storage is returned from 
allocation function "xmalloc".
diffutils-3.7/src/sdiff.c:1166: var_assign: Assigning: "buf" = storage 
returned from "xmalloc(strlen(dir) + 1UL + 5UL + 6UL + 1UL)".
diffutils-3.7/src/sdiff.c:1168: noescape: Resource "buf" is not freed or 
pointed-to in "sprintf". [Note: The source code implementation of the 
function has been overridden by a builtin model.]
diffutils-3.7/src/sdiff.c:1169: noescape: Resource "buf" is not freed or 
pointed-to in "mkstemp".
diffutils-3.7/src/sdiff.c:1172: leaked_storage: Variable "buf" going out 
of scope leaks the storage it points to.
# 1170|     if (0 <= fd)
# 1171|       tmpname = buf;
# 1172|->   return fd;
# 1173|   }

Error: RESOURCE_LEAK (CWE-772): [#def10]
diffutils-3.7/src/util.c:594: alloc_fn: Storage is returned from 
allocation function "xstrdup".
diffutils-3.7/src/util.c:594: var_assign: Assigning: "color_buf" = 
storage returned from "xstrdup(p)".
diffutils-3.7/src/util.c:594: var_assign: Assigning: "buf" = "color_buf".
diffutils-3.7/src/util.c:702: leaked_storage: Variable "buf" going out 
of scope leaks the storage it points to.
diffutils-3.7/src/util.c:702: leaked_storage: Variable "color_buf" going 
out of scope leaks the storage it points to.
#  700|         colors_enabled = false;
#  701|       }
#  702|-> }
#  703|
#  704|   static void