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 <br...@momjian.us> 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 (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers