diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c index 9e707c5d8e..2936050d10 100644 --- a/src/backend/utils/adt/genfile.c +++ b/src/backend/utils/adt/genfile.c @@ -9,7 +9,7 @@ * Author: Andreas Pflug * * 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'; + memcpy(filename, VARDATA(arg), input_len); + filename[input_len] = '\0'; - canonicalize_path(filename); - filename_len = strlen(filename); /* recompute */ + canonicalize_path(filename); /* filename can change length here */ - /* - * 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")))); + /* 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 95e5531c93..ce261d9f05 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 41a526f170..7e37bede7e 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); - trim_directory(path); /* remove directory above */ + pending_strips++; + } + else if (pending_strips > 0 && *spath != '\0') + { + /* trim a regular directory name cancelled by ".." */ + trim_directory(path); + 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; }