From bb099a079d6cbd3fe5a0eab1170ec9e6c91f278b Mon Sep 17 00:00:00 2001 From: Shenhao Wang Date: Sun, 26 Sep 2021 15:44:42 +0800 Subject: [PATCH 2/3] v1 remove function path_contains_parent_reference --- contrib/adminpack/adminpack.c | 6 ---- contrib/adminpack/expected/adminpack.out | 4 +-- contrib/adminpack/sql/adminpack.sql | 2 +- src/backend/utils/adt/genfile.c | 6 ---- src/include/port.h | 1 - src/port/path.c | 36 ++---------------------- 6 files changed, 5 insertions(+), 50 deletions(-) diff --git a/contrib/adminpack/adminpack.c b/contrib/adminpack/adminpack.c index 48c1746910..d5660da669 100644 --- a/contrib/adminpack/adminpack.c +++ b/contrib/adminpack/adminpack.c @@ -88,12 +88,6 @@ convert_and_check_filename(text *arg) */ if (is_absolute_path(filename)) { - /* Disallow '/a/b/data/..' */ - if (path_contains_parent_reference(filename)) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("reference to parent directory (\"..\") not allowed"))); - /* Allow absolute paths if within DataDir */ if (!path_is_prefix_of_path(DataDir, filename)) ereport(ERROR, diff --git a/contrib/adminpack/expected/adminpack.out b/contrib/adminpack/expected/adminpack.out index edf3ebfcba..ad683ba964 100644 --- a/contrib/adminpack/expected/adminpack.out +++ b/contrib/adminpack/expected/adminpack.out @@ -50,8 +50,8 @@ SELECT pg_file_write(current_setting('data_directory') || '/test_file4', 'test4' 5 (1 row) -SELECT pg_file_write(current_setting('data_directory') || '/../test_file4', 'test4', false); -ERROR: reference to parent directory ("..") not allowed +SELECT pg_file_write('abc/../../test_file4', 'test4', false); +ERROR: path must be in or below the current directory RESET ROLE; REVOKE EXECUTE ON FUNCTION pg_file_write(text,text,bool) FROM regress_user1; REVOKE pg_read_all_settings FROM regress_user1; diff --git a/contrib/adminpack/sql/adminpack.sql b/contrib/adminpack/sql/adminpack.sql index 918d0bdc65..87ec827bef 100644 --- a/contrib/adminpack/sql/adminpack.sql +++ b/contrib/adminpack/sql/adminpack.sql @@ -23,7 +23,7 @@ SET ROLE regress_user1; SELECT pg_file_write('../test_file0', 'test0', false); SELECT pg_file_write('/tmp/test_file0', 'test0', false); SELECT pg_file_write(current_setting('data_directory') || '/test_file4', 'test4', false); -SELECT pg_file_write(current_setting('data_directory') || '/../test_file4', 'test4', false); +SELECT pg_file_write('abc/../../test_file4', 'test4', false); RESET ROLE; REVOKE EXECUTE ON FUNCTION pg_file_write(text,text,bool) FROM regress_user1; REVOKE pg_read_all_settings FROM regress_user1; diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c index c436d9318b..1ad7fb644c 100644 --- a/src/backend/utils/adt/genfile.c +++ b/src/backend/utils/adt/genfile.c @@ -71,12 +71,6 @@ convert_and_check_filename(text *arg) */ if (is_absolute_path(filename)) { - /* Disallow '/a/b/data/..' */ - if (path_contains_parent_reference(filename)) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("reference to parent directory (\"..\") not allowed"))); - /* * Allow absolute paths if within DataDir or Log_directory, even * though Log_directory might be outside DataDir. diff --git a/src/include/port.h b/src/include/port.h index 82f63de325..024f2fb21b 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -52,7 +52,6 @@ extern void join_path_components(char *ret_path, extern void canonicalize_path(char *path); extern void make_native_path(char *path); extern void cleanup_path(char *path); -extern bool path_contains_parent_reference(const 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 char *make_absolute_path(const char *path); diff --git a/src/port/path.c b/src/port/path.c index d6741181ba..b32eeb5fde 100644 --- a/src/port/path.c +++ b/src/port/path.c @@ -386,42 +386,10 @@ canonicalize_path(char *path) free(tmppath); } -/* - * 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; -} - /* * 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). + * return false (we only want relative to the cwd). */ bool path_is_relative_and_below_cwd(const char *path) @@ -429,7 +397,7 @@ 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)) + else if (path_is_prefix_of_path("..", path)) return false; #ifdef WIN32 -- 2.26.2