Bruce Momjian wrote:
> > However, it misses the case with for example E:foo, which is a perfectly
> > valid path on windows. Which isn't absolute *or* relative - it's relative
> > to the current directory on the E: drive. Which will be the same as the
> > current directory for the process *if* the process current directory is
> > on drive E:. In other cases, it's a different directory.
>
> I would argue that E:foo is always relative (which matches
> is_absolute_path()). If E: is the current drive of the process, it is
> relative, and if the current drive is not E:, it is relative to the last
> current drive on E: for that process, or the top level if there was no
> current drive. (Tested on XP.)
>
> There seem to be three states:
>
> 1. absolute - already tested by is_absolute_path()
> 2. relative to the current directory (current drive)
> 3. relative on a different drive
>
> We could probably develop code to test all three, but keep in mind that
> the path itself can't distinguish between 2 and 3, and while you can
> test the current drive, if the current drive changes, a 2 could become a
> 3, and via versa.
I have reviewed is_absolute_path() and have implemented
path_is_relative_and_below_cwd() to cleanly handle cases like 'E:abc' on
Win32; patch attached.
--
Bruce Momjian <[email protected]> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/contrib/adminpack/adminpack.c b/contrib/adminpack/adminpack.c
index 381554d..991affd 100644
*** a/contrib/adminpack/adminpack.c
--- b/contrib/adminpack/adminpack.c
*************** convert_and_check_filename(text *arg, bo
*** 73,84 ****
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))
{
/* Allow absolute references within DataDir */
--- 73,78 ----
*************** convert_and_check_filename(text *arg, bo
*** 97,102 ****
--- 91,100 ----
}
else
{
+ if (!path_is_relative_and_below_cwd(filename))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ (errmsg("path must be in or below the current directory"))));
return filename;
}
}
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 93bc401..9774b39 100644
*** a/src/backend/utils/adt/genfile.c
--- b/src/backend/utils/adt/genfile.c
*************** convert_and_check_filename(text *arg)
*** 51,62 ****
filename = text_to_cstring(arg);
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))
{
/* Allow absolute references within DataDir */
--- 51,56 ----
*************** convert_and_check_filename(text *arg)
*** 74,79 ****
--- 68,77 ----
}
else
{
+ if (!path_is_relative_and_below_cwd(filename))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ (errmsg("path must be in or below the current directory"))));
return filename;
}
}
diff --git a/src/include/port.h b/src/include/port.h
index 2020a26..9a14488 100644
*** a/src/include/port.h
--- b/src/include/port.h
*************** extern void join_path_components(char *r
*** 41,47 ****
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 bool path_is_prefix_of_path(const char *path1, const char *path2);
extern const char *get_progname(const char *argv0);
extern void get_share_path(const char *my_exec_path, char *ret_path);
--- 41,47 ----
const char *head, const char *tail);
extern void canonicalize_path(char *path);
extern void make_native_path(char *path);
! extern bool path_is_relative_and_below_cwd(const char *path);
extern bool path_is_prefix_of_path(const char *path1, const char *path2);
extern const char *get_progname(const char *argv0);
extern void get_share_path(const char *my_exec_path, char *ret_path);
*************** extern void pgfnames_cleanup(char **file
*** 77,89 ****
#else
#define IS_DIR_SEP(ch) ((ch) == '/' || (ch) == '\\')
! /*
! * On Win32, a drive letter _not_ followed by a slash, e.g. 'E:abc', is
! * relative to the cwd on that drive, or the drive's root directory
! * if that drive has no cwd. Because the path itself cannot tell us
! * which is the case, we have to assume the worst, i.e. that it is not
! * absolute; this check is done by IS_DIR_SEP(filename[2]).
! */
#define is_absolute_path(filename) \
( \
IS_DIR_SEP((filename)[0]) || \
--- 77,83 ----
#else
#define IS_DIR_SEP(ch) ((ch) == '/' || (ch) == '\\')
! /* See path_is_relative_and_below_cwd() for how we handle 'E:abc'. */
#define is_absolute_path(filename) \
( \
IS_DIR_SEP((filename)[0]) || \
diff --git a/src/port/path.c b/src/port/path.c
index 5b0056d..d3d1249 100644
*** a/src/port/path.c
--- b/src/port/path.c
***************
*** 40,45 ****
--- 40,46 ----
#define IS_PATH_VAR_SEP(ch) ((ch) == ';')
#endif
+ static bool path_contains_parent_reference(const char *path);
static void make_relative_path(char *ret_path, const char *target_path,
const char *bin_path, const char *my_exec_path);
static void trim_directory(char *path);
*************** canonicalize_path(char *path)
*** 336,342 ****
* 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;
--- 337,343 ----
* This is a bit tricky because we mustn't be fooled by "..a.." (legal)
* nor "C:.." (legal on Unix but not Windows).
*/
! static bool
path_contains_parent_reference(const char *path)
{
int path_len;
*************** path_contains_parent_reference(const cha
*** 359,364 ****
--- 360,396 ----
}
/*
+ * Detect whether a path is only in or below the current working directory.
+ * An absolute path that matches the current working directory should
+ * return false (we only want relative to the cwd). We don't allow
+ * "/../" even if that would keep us under the cwd (it is too hard to
+ * track that).
+ */
+ bool
+ path_is_relative_and_below_cwd(const char *path)
+ {
+ if (!is_absolute_path(path))
+ return false;
+ /* don't allow anything above the cwd */
+ else if (path_contains_parent_reference(path))
+ return false;
+ #ifdef WIN32
+ /*
+ * On Win32, a drive letter _not_ followed by a slash, e.g. 'E:abc', is
+ * relative to the cwd on that drive, or the drive's root directory
+ * if that drive has no cwd. Because the path itself cannot tell us
+ * which is the case, we have to assume the worst, i.e. that it is not
+ * below the cwd.
+ */
+ else if (isalpha((unsigned char) path[0]) && path[1] == ':' &&
+ !IS_DIR_SEP(path[2]))
+ return false;
+ #endif
+ else
+ return true;
+ }
+
+ /*
* Detect whether path1 is a prefix of path2 (including equality).
*
* This is pretty trivial, but it seems better to export a function than
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers