Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit b426bd4

Browse files
committed
Simplify coding around path_contains_parent_reference().
Given the existing stipulation that path_contains_parent_reference() must only be invoked on canonicalized paths, we can simplify things in the wake of commit c10f830. It is now only possible to see ".." at the start of a relative path. That means we can simplify path_contains_parent_reference() itself quite a bit, and it makes the two existing outside call sites dead code, since they'd already checked that the path is absolute. We could now fold path_contains_parent_reference() into its only remaining caller path_is_relative_and_below_cwd(). But it seems better to leave it as a separately callable function, in case any extensions are using it. Also document the pre-existing requirement for path_is_relative_and_below_cwd's input to be likewise canonicalized. Shenhao Wang and Tom Lane Discussion: https://postgr.es/m/OSBPR01MB4214FA221FFE046F11F2AD74F2D49@OSBPR01MB4214.jpnprd01.prod.outlook.com
1 parent c10f830 commit b426bd4

File tree

3 files changed

+13
-30
lines changed

3 files changed

+13
-30
lines changed

contrib/adminpack/adminpack.c

-6
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,6 @@ convert_and_check_filename(text *arg)
8888
*/
8989
if (is_absolute_path(filename))
9090
{
91-
/* Disallow '/a/b/data/..' */
92-
if (path_contains_parent_reference(filename))
93-
ereport(ERROR,
94-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
95-
errmsg("reference to parent directory (\"..\") not allowed")));
96-
9791
/* Allow absolute paths if within DataDir */
9892
if (!path_is_prefix_of_path(DataDir, filename))
9993
ereport(ERROR,

src/backend/utils/adt/genfile.c

-6
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,6 @@ convert_and_check_filename(text *arg)
7272
*/
7373
if (is_absolute_path(filename))
7474
{
75-
/* Disallow '/a/b/data/..' */
76-
if (path_contains_parent_reference(filename))
77-
ereport(ERROR,
78-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
79-
errmsg("reference to parent directory (\"..\") not allowed")));
80-
8175
/*
8276
* Allow absolute paths if within DataDir or Log_directory, even
8377
* though Log_directory might be outside DataDir.

src/port/path.c

+13-18
Original file line numberDiff line numberDiff line change
@@ -494,38 +494,33 @@ canonicalize_path(char *path)
494494
* Detect whether a path contains any parent-directory references ("..")
495495
*
496496
* The input *must* have been put through canonicalize_path previously.
497-
*
498-
* This is a bit tricky because we mustn't be fooled by "..a.." (legal)
499-
* nor "C:.." (legal on Unix but not Windows).
500497
*/
501498
bool
502499
path_contains_parent_reference(const char *path)
503500
{
504-
int path_len;
505-
506-
path = skip_drive(path); /* C: shouldn't affect our conclusion */
507-
508-
path_len = strlen(path);
509-
510501
/*
511-
* ".." could be the whole path; otherwise, if it's present it must be at
512-
* the beginning, in the middle, or at the end.
502+
* Once canonicalized, an absolute path cannot contain any ".." at all,
503+
* while a relative path could contain ".."(s) only at the start. So it
504+
* is sufficient to check the start of the path, after skipping any
505+
* Windows drive/network specifier.
513506
*/
514-
if (strcmp(path, "..") == 0 ||
515-
strncmp(path, "../", 3) == 0 ||
516-
strstr(path, "/../") != NULL ||
517-
(path_len >= 3 && strcmp(path + path_len - 3, "/..") == 0))
507+
path = skip_drive(path); /* C: shouldn't affect our conclusion */
508+
509+
if (path[0] == '.' &&
510+
path[1] == '.' &&
511+
(path[2] == '\0' || path[2] == '/'))
518512
return true;
519513

520514
return false;
521515
}
522516

523517
/*
524518
* Detect whether a path is only in or below the current working directory.
519+
*
520+
* The input *must* have been put through canonicalize_path previously.
521+
*
525522
* An absolute path that matches the current working directory should
526-
* return false (we only want relative to the cwd). We don't allow
527-
* "/../" even if that would keep us under the cwd (it is too hard to
528-
* track that).
523+
* return false (we only want relative to the cwd).
529524
*/
530525
bool
531526
path_is_relative_and_below_cwd(const char *path)

0 commit comments

Comments
 (0)