Fix up canonicalize_path to do the right thing in all cases (I think ...
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 12 Aug 2005 21:07:53 +0000 (21:07 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 12 Aug 2005 21:07:53 +0000 (21:07 +0000)
this was harder than it seemed at first glance).  Also push code for
checking for ".." in file names into path.c where it belongs.

src/backend/utils/adt/genfile.c
src/include/port.h
src/port/path.c

index 9e707c5d8e4473718a15314114e6f837d8e650f3..2936050d10529072b7f882dbc824f2064aaf02b9 100644 (file)
@@ -9,7 +9,7 @@
  * Author: Andreas Pflug <pgadmin@pse-consulting.de>
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/adt/genfile.c,v 1.2 2005/08/12 18:23:54 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/adt/genfile.c,v 1.3 2005/08/12 21:07:52 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -46,33 +46,19 @@ typedef struct
 static char *
 check_and_make_absolute(text *arg)
 {
-       int filename_len = VARSIZE(arg) - VARHDRSZ;
-       char *filename = palloc(filename_len + 1);
+       int input_len = VARSIZE(arg) - VARHDRSZ;
+       char *filename = palloc(input_len + 1);
        
-       memcpy(filename, VARDATA(arg), filename_len);
-       filename[filename_len] = '\0';
-
-       canonicalize_path(filename);
-       filename_len = strlen(filename);        /* recompute */
-
-       /*
-        *      Prevent reference to the parent directory.
-        *      "..a.." is a valid file name though.
-        *
-        * XXX this is BROKEN because it fails to prevent "C:.." on Windows.
-        * Need access to "skip_drive" functionality to do it right.  (There
-        * is no actual security hole because we'll prepend the DataDir below,
-        * resulting in a just-plain-broken path, but we should give the right
-        * error message instead.)
-        */
-       if (strcmp(filename, "..") == 0 ||                                              /* whole */
-               strncmp(filename, "../", 3) == 0 ||                                     /* beginning */
-               strstr(filename, "/../") != NULL ||                                     /* middle */
-               (filename_len >= 3 &&
-                strcmp(filename + filename_len - 3, "/..") == 0))      /* end */
-                       ereport(ERROR,
-                                 (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-                                  (errmsg("reference to parent directory (\"..\") not allowed"))));
+       memcpy(filename, VARDATA(arg), input_len);
+       filename[input_len] = '\0';
+
+       canonicalize_path(filename);    /* filename can change length here */
+
+       /* Disallow ".." in the path */
+       if (path_contains_parent_reference(filename))
+               ereport(ERROR,
+                               (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                                (errmsg("reference to parent directory (\"..\") not allowed"))));
 
        if (is_absolute_path(filename))
        {
@@ -90,7 +76,7 @@ check_and_make_absolute(text *arg)
        }
        else
        {
-           char *absname = palloc(strlen(DataDir) + filename_len + 2);
+           char *absname = palloc(strlen(DataDir) + strlen(filename) + 2);
                sprintf(absname, "%s/%s", DataDir, filename);
                pfree(filename);
                return absname;
index 95e5531c931430091df22d00a429daf1e6440835..ce261d9f050a357f15758cd1d81a0c4d147f11b3 100644 (file)
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/port.h,v 1.80 2005/08/02 19:02:32 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/port.h,v 1.81 2005/08/12 21:07:52 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -32,6 +32,7 @@ extern void join_path_components(char *ret_path,
                                                                 const char *head, const char *tail);
 extern void canonicalize_path(char *path);
 extern void make_native_path(char *path);
+extern bool path_contains_parent_reference(const char *path);
 extern const char *get_progname(const char *argv0);
 extern void get_share_path(const char *my_exec_path, char *ret_path);
 extern void get_etc_path(const char *my_exec_path, char *ret_path);
index 41a526f170f89676a08469666636f26018ad98e4..7e37bede7eaaeaebed51f3ea7f666c1997d354d2 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/port/path.c,v 1.56 2005/08/12 19:43:32 momjian Exp $
+ *       $PostgreSQL: pgsql/src/port/path.c,v 1.57 2005/08/12 21:07:53 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -225,7 +225,9 @@ void
 canonicalize_path(char *path)
 {
        char       *p, *to_p;
+       char       *spath;
        bool            was_sep = false;
+       int                     pending_strips;
 
 #ifdef WIN32
        /*
@@ -277,30 +279,89 @@ canonicalize_path(char *path)
 
        /*
         * Remove any trailing uses of "." and process ".." ourselves
+        *
+        * Note that "/../.." should reduce to just "/", while "../.." has to
+        * be kept as-is.  In the latter case we put back mistakenly trimmed
+        * ".." components below.  Also note that we want a Windows drive spec
+        * to be visible to trim_directory(), but it's not part of the logic
+        * that's looking at the name components; hence distinction between
+        * path and spath.
         */
+       spath = skip_drive(path);
+       pending_strips = 0;
        for (;;)
        {
-               int                     len = strlen(path);
+               int                     len = strlen(spath);
 
-               if (len > 2 && strcmp(path + len - 2, "/.") == 0)
+               if (len >= 2 && strcmp(spath + len - 2, "/.") == 0)
                        trim_directory(path);
-               /*
-                *      Process only a single trailing "..", and only if ".." does
-                *      not preceed it.
-                *      So, we only deal with "/usr/local/..", not with "/usr/local/../..".
-                *      We don't handle the even more complex cases, like
-                *      "usr/local/../../..".
-                */
-               else if (len > 3 && strcmp(path + len - 3, "/..") == 0 &&
-                                (len != 5 || strcmp(path, "../..") != 0) &&
-                                (len < 6 || strcmp(path + len - 6, "/../..") != 0))
+               else if (strcmp(spath, ".") == 0)
+               {
+                       /* Want to leave "." alone, but "./.." has to become ".." */
+                       if (pending_strips > 0)
+                               *spath = '\0';
+                       break;
+               }
+               else if ((len >= 3 && strcmp(spath + len - 3, "/..") == 0) ||
+                                strcmp(spath, "..") == 0)
+               {
+                       trim_directory(path);
+                       pending_strips++;
+               }
+               else if (pending_strips > 0 && *spath != '\0')
                {
+                       /* trim a regular directory name cancelled by ".." */
                        trim_directory(path);
-                       trim_directory(path);   /* remove directory above */
+                       pending_strips--;
+                       /* foo/.. should become ".", not empty */
+                       if (*spath == '\0')
+                               strcpy(spath, ".");
                }
                else
                        break;
        }
+
+       if (pending_strips > 0)
+       {
+               /*
+                * We could only get here if path is now totally empty (other than
+                * a possible drive specifier on Windows).
+                * We have to put back one or more ".."'s that we took off.
+                */
+               while (--pending_strips > 0)
+                       strcat(path, "../");
+               strcat(path, "..");
+       }
+}
+
+/*
+ * Detect whether a path contains any parent-directory references ("..")
+ *
+ * The input *must* have been put through canonicalize_path previously.
+ *
+ * This is a bit tricky because we mustn't be fooled by "..a.." (legal)
+ * nor "C:.." (legal on Unix but not Windows).
+ */
+bool
+path_contains_parent_reference(const char *path)
+{
+       int             path_len;
+
+       path = skip_drive(path);        /* C: shouldn't affect our conclusion */
+
+       path_len = strlen(path);
+
+       /*
+        * ".." could be the whole path; otherwise, if it's present it must
+        * be at the beginning, in the middle, or at the end.
+        */
+       if (strcmp(path, "..") == 0 ||
+               strncmp(path, "../", 3) == 0 ||
+               strstr(path, "/../") != NULL ||
+               (path_len >= 3 && strcmp(path + path_len - 3, "/..") == 0))
+               return true;
+
+       return false;
 }