From c481b6a27fbd3e28cc953b58131df9f8ff0505c6 Mon Sep 17 00:00:00 2001 From: Lutz Justen Date: Mon, 6 Mar 2023 15:25:49 +0100 Subject: [PATCH] [common] Prevent command execution in ExpandPathVariables (#87) Command execution is not something users would expect. Even though there is no security issue (right now), it's probably better to turn it off. --- common/path.cc | 5 ++++- common/path.h | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/common/path.cc b/common/path.cc index e4a54c3..e75885a 100644 --- a/common/path.cc +++ b/common/path.cc @@ -219,9 +219,12 @@ absl::Status ExpandPathVariables(std::string* path) { *path = Util::WideToUtf8Str(wchar_expanded); return absl::OkStatus(); #else + // Exclude command substitution. It.s not what users of this method would + // expect and could lead to security issues. wordexp_t res; - wordexp(path->c_str(), &res, 0); + wordexp(path->c_str(), &res, WRDE_NOCMD); if (res.we_wordc > 1) { + wordfree(&res); return absl::InvalidArgumentError( "Path expands to multiple results (did you use * etc. ?"); } diff --git a/common/path.h b/common/path.h index 204580f..ff2ec22 100644 --- a/common/path.h +++ b/common/path.h @@ -104,8 +104,8 @@ absl::Status GetKnownFolderPath(FolderId folder_id, std::string* path); // Expands environment path variables like %APPDATA% on Windows or ~ on Linux. // On Windows, variables are matched case invariantly. Unknown environment // variables are not changed. -// On Linux, performs a shell-like expansion. Returns an error if multiple -// results would be returned, e.g. from *.txt. +// On Linux, performs a shell-like expansion, but without command substitution. +// Returns an error if multiple results would be returned, e.g. from *.txt. absl::Status ExpandPathVariables(std::string* path); // Returns the environment variable with given |name| in |value|.