From 81cce8ded58ed2f2e6ef2227509a90ad60d63502 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 17 Jan 2018 11:13:46 +0100 Subject: [PATCH 1/5] path-util: do something useful if the prefix is "" in path_make_absolute() Do not insert a "/" if the prefix we shall use is empty. It's a corner case we should probably take care of. --- src/basic/path-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index ab4778d4ed..b877bdc1cb 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -81,7 +81,7 @@ char *path_make_absolute(const char *p, const char *prefix) { /* Makes every item in the list an absolute path by prepending * the prefix, if specified and necessary */ - if (path_is_absolute(p) || !prefix) + if (path_is_absolute(p) || isempty(prefix)) return strdup(p); return strjoin(prefix, "/", p); From cddd2ce1069c4d84179ce606e830db98abc50609 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 17 Jan 2018 11:15:00 +0100 Subject: [PATCH 2/5] path-util: don't add extra "/" when prefix already is suffixed by slash No need to insert duplicate "/" if we can avoid it. This is particularly relevant if the prefix passed in is the root directory. --- src/basic/path-util.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index b877bdc1cb..f5f506ccf5 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -84,7 +84,10 @@ char *path_make_absolute(const char *p, const char *prefix) { if (path_is_absolute(p) || isempty(prefix)) return strdup(p); - return strjoin(prefix, "/", p); + if (endswith(prefix, "/")) + return strjoin(prefix, p); + else + return strjoin(prefix, "/", p); } int path_make_absolute_cwd(const char *p, char **ret) { From a2556d25ae8e7c1aa1e75affb45226b02813a03b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 17 Jan 2018 11:16:31 +0100 Subject: [PATCH 3/5] path-util: introduce new safe_getcwd() wrapper It's like get_current_dir_name() but protects us from CVE-2018-1000001-style exploits: https://www.halfdog.net/Security/2017/LibcRealpathBufferUnderflow/ --- src/basic/path-util.c | 18 ++++++++++++++++++ src/basic/path-util.h | 1 + 2 files changed, 19 insertions(+) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index f5f506ccf5..fbf69d12eb 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -90,6 +90,24 @@ char *path_make_absolute(const char *p, const char *prefix) { return strjoin(prefix, "/", p); } +int safe_getcwd(char **ret) { + char *cwd; + + cwd = get_current_dir_name(); + if (!cwd) + return negative_errno(); + + /* Let's make sure the directory is really absolute, to protect us from the logic behind + * CVE-2018-1000001 */ + if (cwd[0] != '/') { + free(cwd); + return -ENOMEDIUM; + } + + *ret = cwd; + return 0; +} + int path_make_absolute_cwd(const char *p, char **ret) { char *c; diff --git a/src/basic/path-util.h b/src/basic/path-util.h index f79cdf928e..89c285e076 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -41,6 +41,7 @@ bool is_path(const char *p) _pure_; int path_split_and_make_absolute(const char *p, char ***ret); bool path_is_absolute(const char *p) _pure_; char* path_make_absolute(const char *p, const char *prefix); +int safe_getcwd(char **ret); int path_make_absolute_cwd(const char *p, char **ret); int path_make_relative(const char *from_dir, const char *to_path, char **_r); char* path_kill_slashes(char *path); From d72495759b0f953b86f16669bf8120ede28836a5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 17 Jan 2018 11:17:38 +0100 Subject: [PATCH 4/5] tree-wide: port all code to use safe_getcwd() --- src/basic/path-util.c | 7 ++++--- src/cgls/cgls.c | 6 +++--- src/nspawn/nspawn.c | 7 +++++-- src/test/test-fs-util.c | 2 +- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index fbf69d12eb..666f48cfc3 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -110,6 +110,7 @@ int safe_getcwd(char **ret) { int path_make_absolute_cwd(const char *p, char **ret) { char *c; + int r; assert(p); assert(ret); @@ -122,9 +123,9 @@ int path_make_absolute_cwd(const char *p, char **ret) { else { _cleanup_free_ char *cwd = NULL; - cwd = get_current_dir_name(); - if (!cwd) - return negative_errno(); + r = safe_getcwd(&cwd); + if (r < 0) + return r; c = strjoin(cwd, "/", p); } diff --git a/src/cgls/cgls.c b/src/cgls/cgls.c index fb44b9f669..bd8c6a0059 100644 --- a/src/cgls/cgls.c +++ b/src/cgls/cgls.c @@ -277,9 +277,9 @@ int main(int argc, char *argv[]) { if (!arg_machine) { _cleanup_free_ char *cwd = NULL; - cwd = get_current_dir_name(); - if (!cwd) { - r = log_error_errno(errno, "Cannot determine current working directory: %m"); + r = safe_getcwd(&cwd); + if (r < 0) { + log_error_errno(r, "Cannot determine current working directory: %m"); goto finish; } diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index f580b46f86..0f05ecff03 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -2163,8 +2163,11 @@ static int determine_names(void) { if (!arg_ephemeral) arg_read_only = arg_read_only || i->read_only; - } else - arg_directory = get_current_dir_name(); + } else { + r = safe_getcwd(&arg_directory); + if (r < 0) + return log_error_errno(r, "Failed to determine current directory: %m"); + } if (!arg_directory && !arg_image) { log_error("Failed to determine path, please use -D or -i."); diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index cda6d9cc72..9f3a500080 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -326,7 +326,7 @@ static void test_readlink_and_make_absolute(void) { free(r); assert_se(unlink(name_alias) >= 0); - assert_se(pwd = get_current_dir_name()); + assert_se(safe_getcwd(&pwd) >= 0); assert_se(chdir(tempdir) >= 0); assert_se(symlink(name2, name_alias) >= 0); From 7aeeb313ad35ff82eee67b568866e95cf22725c2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 17 Jan 2018 11:17:55 +0100 Subject: [PATCH 5/5] path-util: don't insert duplicate "/" in path_make_absolute_cwd() When the working directory is "/" it's prettier not to insert a second "/" in the path, even though it is technically correct. --- src/basic/path-util.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 666f48cfc3..df94629385 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -127,7 +127,10 @@ int path_make_absolute_cwd(const char *p, char **ret) { if (r < 0) return r; - c = strjoin(cwd, "/", p); + if (endswith(cwd, "/")) + c = strjoin(cwd, p); + else + c = strjoin(cwd, "/", p); } if (!c) return -ENOMEM;