From de661c41a1e7e52296c91b9caa0bff8e4885c751 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Thu, 22 Oct 2020 14:06:53 +0200 Subject: [PATCH 1/4] common: Fix infloop in p11_path_build If p11_path_build is called with 2 or more arguments and the non-first argument is an empty string (""), it previously fell into an infloop. Reported by Karel Srot. --- common/path.c | 4 +++- common/test-path.c | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/common/path.c b/common/path.c index 17a6230..53d394f 100644 --- a/common/path.c +++ b/common/path.c @@ -241,8 +241,10 @@ p11_path_build (const char *path, num--; if (at != 0) { - if (num == 0) + if (num == 0) { + path = va_arg (va, const char *); continue; + } built[at++] = delim; } diff --git a/common/test-path.c b/common/test-path.c index 2eb5444..f137a0c 100644 --- a/common/test-path.c +++ b/common/test-path.c @@ -88,6 +88,8 @@ static void test_build (void) { #ifdef OS_UNIX + assert_str_eq_free ("/root", + p11_path_build ("/root", "", NULL)); assert_str_eq_free ("/root/second", p11_path_build ("/root", "second", NULL)); assert_str_eq_free ("/root/second", @@ -99,6 +101,8 @@ test_build (void) assert_str_eq_free ("/root/second/third", p11_path_build ("/root", "/second/third", NULL)); #else /* OS_WIN32 */ + assert_str_eq_free ("C:\\root", + p11_path_build ("C:\\root", "", NULL)); assert_str_eq_free ("C:\\root\\second", p11_path_build ("C:\\root", "second", NULL)); assert_str_eq_free ("C:\\root\\second", -- 2.26.2 From 1eac9a1c41828d5da4b640746e0002c7ab964e8e Mon Sep 17 00:00:00 2001 From: Alexander Sosedkin Date: Tue, 27 Oct 2020 11:08:53 +0100 Subject: [PATCH 2/4] Remove more duplicate separators in p11_path_build Makes p11_path_build remove duplicate separators more thoroughly, e.g., after a "" or in the first argument. --- common/path.c | 26 +++++++++++++++++++------- common/test-path.c | 22 ++++++++++++++++++++++ 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/common/path.c b/common/path.c index 53d394f..0ad176c 100644 --- a/common/path.c +++ b/common/path.c @@ -94,15 +94,21 @@ p11_path_base (const char *path) } static inline bool -is_path_component_or_null (char ch) +is_path_component (char ch) { - return (ch == '\0' || ch == '/' + return (ch == '/' #ifdef OS_WIN32 || ch == '\\' #endif ); } +static inline bool +is_path_component_or_null (char ch) +{ + return is_path_component (ch) || ch == '\0'; +} + static char * expand_homedir (const char *remainder) { @@ -235,6 +241,15 @@ p11_path_build (const char *path, while (path != NULL) { num = strlen (path); + /* Trim beginning of path */ + while (is_path_component (path[0])) { + /* But preserve the leading path component */ + if (!at && !is_path_component (path[1])) + break; + path++; + num--; + } + /* Trim end of the path */ until = (at > 0) ? 0 : 1; while (num > until && is_path_component_or_null (path[num - 1])) @@ -245,7 +260,8 @@ p11_path_build (const char *path, path = va_arg (va, const char *); continue; } - built[at++] = delim; + if (built[at - 1] != delim) + built[at++] = delim; } assert (at + num < len); @@ -253,10 +269,6 @@ p11_path_build (const char *path, at += num; path = va_arg (va, const char *); - - /* Trim beginning of path */ - while (path && path[0] && is_path_component_or_null (path[0])) - path++; } va_end (va); diff --git a/common/test-path.c b/common/test-path.c index f137a0c..cf4a8e3 100644 --- a/common/test-path.c +++ b/common/test-path.c @@ -88,6 +88,16 @@ static void test_build (void) { #ifdef OS_UNIX + assert_str_eq_free ("/", + p11_path_build ("/", NULL)); + assert_str_eq_free ("/", + p11_path_build ("", "//", NULL)); + assert_str_eq_free ("/root", + p11_path_build ("///root///", NULL)); + assert_str_eq_free ("/root", + p11_path_build ("/", "root", NULL)); + assert_str_eq_free ("/root", + p11_path_build ("", "/root", NULL)); assert_str_eq_free ("/root", p11_path_build ("/root", "", NULL)); assert_str_eq_free ("/root/second", @@ -96,11 +106,19 @@ test_build (void) p11_path_build ("/root", "/second", NULL)); assert_str_eq_free ("/root/second", p11_path_build ("/root/", "second", NULL)); + assert_str_eq_free ("/root/second", + p11_path_build ("/root//", "//second/", NULL)); + assert_str_eq_free ("/root/second", + p11_path_build ("/root//", "", "//second/", NULL)); assert_str_eq_free ("/root/second/third", p11_path_build ("/root", "second", "third", NULL)); assert_str_eq_free ("/root/second/third", p11_path_build ("/root", "/second/third", NULL)); #else /* OS_WIN32 */ + assert_str_eq_free ("C:\\root", + p11_path_build ("C:\\", "root", NULL)); + assert_str_eq_free ("C:\\root", + p11_path_build ("", "C:\\root", NULL)); assert_str_eq_free ("C:\\root", p11_path_build ("C:\\root", "", NULL)); assert_str_eq_free ("C:\\root\\second", @@ -109,6 +127,10 @@ test_build (void) p11_path_build ("C:\\root", "\\second", NULL)); assert_str_eq_free ("C:\\root\\second", p11_path_build ("C:\\root\\", "second", NULL)); + assert_str_eq_free ("C:\\root\\second", + p11_path_build ("C:\\root\\\\", "\\\\second", NULL)); + assert_str_eq_free ("C:\\root\\second", + p11_path_build ("C:\\root\\\\", "", "\\\\second", NULL)); assert_str_eq_free ("C:\\root\\second\\third", p11_path_build ("C:\\root", "second", "third", NULL)); assert_str_eq_free ("C:\\root\\second/third", -- 2.26.2 From e5a1f444b7d299e77dd57862f3cc5783e697a10e Mon Sep 17 00:00:00 2001 From: Alexander Sosedkin Date: Tue, 27 Oct 2020 13:33:34 +0100 Subject: [PATCH 3/4] Use is_path_component in one more place --- common/path.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/path.c b/common/path.c index 0ad176c..8f57ec6 100644 --- a/common/path.c +++ b/common/path.c @@ -119,7 +119,7 @@ expand_homedir (const char *remainder) return NULL; } - while (remainder[0] && is_path_component_or_null (remainder[0])) + while (is_path_component (remainder[0])) remainder++; if (remainder[0] == '\0') remainder = NULL; -- 2.26.2 From ce66cf00b6b207c1d452af23cb062ca0adf57dac Mon Sep 17 00:00:00 2001 From: Alexander Sosedkin Date: Tue, 27 Oct 2020 16:01:32 +0100 Subject: [PATCH 4/4] Rename is_path_component to is_path_separator Thanks to Daiki Ueno for noticing the misnaming. --- common/path.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/common/path.c b/common/path.c index 8f57ec6..d0d1893 100644 --- a/common/path.c +++ b/common/path.c @@ -94,7 +94,7 @@ p11_path_base (const char *path) } static inline bool -is_path_component (char ch) +is_path_separator (char ch) { return (ch == '/' #ifdef OS_WIN32 @@ -104,9 +104,9 @@ is_path_component (char ch) } static inline bool -is_path_component_or_null (char ch) +is_path_separator_or_null (char ch) { - return is_path_component (ch) || ch == '\0'; + return is_path_separator (ch) || ch == '\0'; } static char * @@ -119,7 +119,7 @@ expand_homedir (const char *remainder) return NULL; } - while (is_path_component (remainder[0])) + while (is_path_separator (remainder[0])) remainder++; if (remainder[0] == '\0') remainder = NULL; @@ -127,7 +127,7 @@ expand_homedir (const char *remainder) /* Expand $XDG_CONFIG_HOME */ if (remainder != NULL && strncmp (remainder, ".config", 7) == 0 && - is_path_component_or_null (remainder[7])) { + is_path_separator_or_null (remainder[7])) { env = getenv ("XDG_CONFIG_HOME"); if (env && env[0]) return p11_path_build (env, remainder + 8, NULL); @@ -180,7 +180,7 @@ p11_path_expand (const char *path) return_val_if_fail (path != NULL, NULL); if (strncmp (path, "~", 1) == 0 && - is_path_component_or_null (path[1])) { + is_path_separator_or_null (path[1])) { return expand_homedir (path + 1); } else { @@ -242,9 +242,9 @@ p11_path_build (const char *path, num = strlen (path); /* Trim beginning of path */ - while (is_path_component (path[0])) { + while (is_path_separator (path[0])) { /* But preserve the leading path component */ - if (!at && !is_path_component (path[1])) + if (!at && !is_path_separator (path[1])) break; path++; num--; @@ -252,7 +252,7 @@ p11_path_build (const char *path, /* Trim end of the path */ until = (at > 0) ? 0 : 1; - while (num > until && is_path_component_or_null (path[num - 1])) + while (num > until && is_path_separator_or_null (path[num - 1])) num--; if (at != 0) { @@ -288,17 +288,17 @@ p11_path_parent (const char *path) /* Find the end of the last component */ e = path + strlen (path); - while (e != path && is_path_component_or_null (*e)) + while (e != path && is_path_separator_or_null (*e)) e--; /* Find the beginning of the last component */ - while (e != path && !is_path_component_or_null (*e)) { + while (e != path && !is_path_separator_or_null (*e)) { had = true; e--; } /* Find the end of the last component */ - while (e != path && is_path_component_or_null (*e)) + while (e != path && is_path_separator_or_null (*e)) e--; if (e == path) { @@ -327,7 +327,7 @@ p11_path_prefix (const char *string, return a > b && strncmp (string, prefix, b) == 0 && - is_path_component_or_null (string[b]); + is_path_separator_or_null (string[b]); } void -- 2.26.2