3 files changed, 91 insertions, 43 deletions

diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 9e707c5d8e4..2936050d105 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/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;
diff --git a/src/include/port.h b/src/include/port.h
index 95e5531c931..ce261d9f050 100644
--- a/src/include/port.h
+++ b/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);
diff --git a/src/port/path.c b/src/port/path.c
index 41a526f170f..7e37bede7ea 100644
--- a/src/port/path.c
+++ b/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;
 }