Fix up canonicalize_path to do the right thing in all cases (I think ...

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

@@ -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;

src/include/port.h

@@ -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);

src/port/path.c

@@ -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;
 }