From 0d1ebe0194e79c8a211574f61740c4235a1fc30a Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 12 Aug 2005 21:07:53 +0000
Subject: [PATCH] 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 | 42 ++++++----------
src/include/port.h | 3 +-
src/port/path.c | 89 +++++++++++++++++++++++++++------
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;
}
--
GitLab