Fix use-after-free bug in name_is_valid()

Reported by Steffen Sledz in
https://bugzilla.novell.com/show_bug.cgi?id=755136 via Jean Delvare.
Bug introduced in commit v2.6.1-115-ge0f7075; fixed with help from Jim
Meyering <meyering@redhat.com>.
* src/common.h (ARRAY_SIZE): New macro.
* src/pch.c (invalid_names): New global variable for remembering bad names.
(intuit_diff_type): Reset invalid_names for each new patch; the names from the
previous patch have already been freed.
(name_is_valid): Use invalid_names.  Make the code "safer" and avoid
duplication.

2 files changed, 32 insertions, 21 deletions

diff --git a/src/common.h b/src/common.h
index d91f696..bc93e59 100644
--- a/src/common.h
+++ b/src/common.h
@@ -62,6 +62,7 @@
 
 #define strEQ(s1,s2) (!strcmp(s1, s2))
 #define strnEQ(s1,s2,l) (!strncmp(s1, s2, l))
+#define ARRAY_SIZE(a) (sizeof (a) / sizeof ((a)[0]))
 
 /* typedefs */
 
diff --git a/src/pch.c b/src/pch.c
index 6b0605e..a07f291 100644
--- a/src/pch.c
+++ b/src/pch.c
@@ -42,6 +42,7 @@ static int p_says_nonexistent[2];	/* [0] for old file, [1] for new:
 		2 for nonexistent */
 static int p_rfc934_nesting;		/* RFC 934 nesting level */
 static char *p_name[3];			/* filenames in patch headers */
+static char const *invalid_names[2];
 bool p_copy[2];				/* Does this patch create a copy? */
 bool p_rename[2];			/* Does this patch rename a file? */
 static char *p_timestr[2];		/* timestamps as strings */
@@ -379,34 +380,41 @@ skip_hex_digits (char const *str)
 static bool
 name_is_valid (char const *name)
 {
-  static char const *bad[2];
   char const *n;
+  int i;
+  bool is_valid = true;
 
-  if (bad[0] && ! strcmp (bad[0], name))
-    return false;
-  if (bad[1] && ! strcmp (bad[1], name))
-    return false;
+  for (i = 0; i < ARRAY_SIZE (invalid_names); i++)
+    {
+      if (! invalid_names[i])
+	break;
+      if (! strcmp (invalid_names[i], name))
+	return false;
+    }
 
   if (IS_ABSOLUTE_FILE_NAME (name))
+    is_valid = false;
+  else
+    for (n = name; *n; )
+      {
+	if (*n == '.' && *++n == '.' && ( ! *++n || ISSLASH (*n)))
+	  {
+	    is_valid = false;
+	    break;
+	  }
+	while (*n && ! ISSLASH (*n))
+	  n++;
+	while (ISSLASH (*n))
+	  n++;
+      }
+
+  if (! is_valid)
     {
       say ("Ignoring potentially dangerous file name %s\n", quotearg (name));
-      bad[!! bad[0]] = name;
-      return false;
-    }
-  for (n = name; *n; )
-    {
-      if (*n == '.' && *++n == '.' && ( ! *++n || ISSLASH (*n)))
-        {
-	  say ("Ignoring potentially dangerous file name %s\n", quotearg (name));
-	  bad[!! bad[0]] = name;
-	  return false;
-	}
-      while (*n && ! ISSLASH (*n))
-	n++;
-      while (ISSLASH (*n))
-	n++;
+      if (i < ARRAY_SIZE (invalid_names))
+	invalid_names[i] = name;
     }
-  return true;
+  return is_valid;
 }
 
 /* Determine what kind of diff is in the remaining part of the patch file. */
@@ -434,6 +442,8 @@ intuit_diff_type (bool need_header, mode_t *p_file_type)
 	  free (p_name[i]);
 	  p_name[i] = 0;
         }
+    for (i = 0; i < ARRAY_SIZE (invalid_names); i++)
+	invalid_names[i] = NULL;
     for (i = OLD; i <= NEW; i++)
       if (p_timestr[i])
 	{