Re: [bug-patch] [PATCH] do not validate target name when it is specified on the command line

[Andreas Gruenbacher]

On Wednesday 16 February 2011 15:27:42 Jim Meyering wrote:
> Patch still stats the offending file.

That would be tricky to avoid while at the same time not warning about file
names that patch is not going to use anyway.  I don't think it can do any
harm.

> [...] if you create ../z, then it will print the new "Ignoring ..."
> diagnostic twice.

That's ugly, and I did not run into this case before.
Here's a possible fix.

Andreas


diff --git a/src/pch.c b/src/pch.c
index 41c15b6..88210c2 100644
--- a/src/pch.c
+++ b/src/pch.c
@@ -74,7 +74,7 @@ static char *p_c_function;            /* the C function a 
hunk is in */
 
 static char *scan_linenum (char *, lin *);
 static enum diff intuit_diff_type (bool, mode_t *);
-static enum nametype best_name (char * const *, int const *);
+static enum nametype best_name (char **, int const *);
 static int prefix_components (char *, bool);
 static size_t pget_line (size_t, int, bool, bool);
 static size_t get_line (void);
@@ -377,21 +377,21 @@ skip_hex_digits (char const *str)
 }
 
 static bool
-name_is_valid (char const *name)
+name_is_valid (char **name)
 {
-  const char *n = name;
+  const char *n;
 
-  if (IS_ABSOLUTE_FILE_NAME (name))
+  if (IS_ABSOLUTE_FILE_NAME (*name))
     {
-      say ("Ignoring potentially dangerous file name %s\n", quotearg (name));
-      return false;
+      say ("Ignoring potentially dangerous file name %s\n", quotearg (*name));
+      goto fail;
     }
-  for (n = name; *n; )
+  for (n = *name; *n; )
     {
       if (*n == '.' && *++n == '.' && ( ! *++n || ISSLASH (*n)))
         {
-         say ("Ignoring potentially dangerous file name %s\n", quotearg 
(name));
-         return false;
+         say ("Ignoring potentially dangerous file name %s\n", quotearg 
(*name));
+         goto fail;
        }
       while (*n && ! ISSLASH (*n))
        n++;
@@ -399,6 +399,11 @@ name_is_valid (char const *name)
        n++;
     }
   return true;
+
+fail:
+  free (*name);
+  *name = 0;
+  return false;
 }
 
 /* Determine what kind of diff is in the remaining part of the patch file. */
@@ -854,7 +859,7 @@ intuit_diff_type (bool need_header, mode_t *p_file_type)
              else
                {
                  stat_errno[i] = 0;
-                 if (posixly_correct && name_is_valid (p_name[i]))
+                 if (posixly_correct && name_is_valid (&p_name[i]))
                    break;
                }
              i0 = i;
@@ -992,7 +997,7 @@ prefix_components (char *filename, bool checkdirs)
    Ignore null names, and ignore NAME[i] if IGNORE[i] is nonzero.
    Return NONE if all names are ignored.  */
 static enum nametype
-best_name (char *const *name, int const *ignore)
+best_name (char **name, int const *ignore)
 {
   enum nametype i;
   int components[3];
@@ -1027,10 +1032,10 @@ best_name (char *const *name, int const *ignore)
   /* Of those, take the first name.  */
   for (i = OLD;  i <= INDEX;  i++)
     if (name[i] && !ignore[i]
-       && name_is_valid (name[i])
        && components[i] == components_min
        && basename_len[i] == basename_len_min
-       && len[i] == len_min)
+       && len[i] == len_min
+       && name_is_valid (&name[i]))
       break;
 
   return i;
diff --git a/tests/bad-filenames b/tests/bad-filenames
index 0bc23eb..d86ce22 100644
--- a/tests/bad-filenames
+++ b/tests/bad-filenames
@@ -114,3 +114,27 @@ echo 1 > g
 check 'patch -f -p1 --dry-run < d.diff || echo status: $?' <<EOF
 patching file g
 EOF
+
+mkdir d
+cd d
+cat > d.diff <<EOF
+--- /dev/null
++++ ../h
+@@ -0,0 +1 @@
++x
+EOF
+
+touch ../h
+check 'patch -f -p0 < d.diff || echo status: $?' <<EOF
+Ignoring potentially dangerous file name ../h
+can't find file to patch at input line 3
+Perhaps you used the wrong -p or --strip option?
+The text leading up to this was:
+--------------------------
+|--- /dev/null
+|+++ ../h
+--------------------------
+No file to patch.  Skipping patch.
+1 out of 1 hunk ignored
+status: 1
+EOF