diff --git a/cf-agent/verify_packages.c b/cf-agent/verify_packages.c index a98a24e370..e68e423eba 100644 --- a/cf-agent/verify_packages.c +++ b/cf-agent/verify_packages.c @@ -54,6 +54,7 @@ #include #include #include +#include /* Called structure: @@ -104,6 +105,7 @@ typedef enum PACKAGE_PROMISE_TYPE_NEW_ERROR } PackagePromiseType; +static bool SanitizePackagePromiser(EvalContext *ctx, const Attributes *a, const Promise *pp); static bool PackageSanityCheck(EvalContext *ctx, const Attributes *a, const Promise *pp); static bool VerifyInstalledPackages(EvalContext *ctx, PackageManager **alllists, const char *default_arch, const Attributes *a, const Promise *pp, PromiseResult *result); @@ -186,6 +188,12 @@ PromiseResult VerifyPackagesPromise(EvalContext *ctx, const Promise *pp) PackagePromiseType package_promise_type = GetPackagePromiseVersion(&a.packages, &a.new_packages); + if (!SanitizePackagePromiser(ctx, &a, pp)) + { + Log(LOG_LEVEL_VERBOSE, "Package promise %s failed sanitization check", pp->promiser); + return PROMISE_RESULT_FAIL; + } + switch (package_promise_type) { case PACKAGE_PROMISE_TYPE_NEW: @@ -368,6 +376,51 @@ static PromiseResult HandleOldPackagePromiseType(EvalContext *ctx, const Promise return result; } +static bool SanitizePackagePromiser(EvalContext *ctx, const Attributes *a, const Promise *pp) +{ + assert(a != NULL); + assert(pp != NULL); + + const char *promiser = pp->promiser; + + /* Shell metacharacters that could be used for command injection + * + * Here's the rationale for each shell metacharacter in the list: + * ; Command separator, allows running additional commands + * | Pipe, can redirect output to another command + * & Background execution or command chaining (`&&`) + * ` Command substitution (backticks execute enclosed command) + * $ Variable expansion and command substitution (`$(...)`) + * ( Subshell execution, command grouping + * < Input/output redirection, can overwrite files + * { Brace expansion, command grouping + * [ Character class matching + * * Wildcard, matches any characters + * ? Wildcard, matches single character + * ~ Home directory expansion + * \ Escape character, can break out of quoting + * ' Quotes, can break parsing if unbalanced + * ! History expansion in some shells + * # Comment character, can truncate commands + * \n \r Newlines can inject additional commands + */ + const char *shell_metacharacters = ";|&`$(){}[]<>!#*?~\\'\"\n\r"; + + if (a->packages.package_commands_useshell) + { + const char *bad_char = strpbrk(promiser, shell_metacharacters); + if (bad_char != NULL) + { + cfPS_HELPER_2ARG(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_FAIL, pp, a, + "Package promiser '%s' contains shell metacharacter '%c' which could allow command injection", + promiser, *bad_char); + return false; + } + } + + return true; +} + /** @brief Pre-check of promise contents diff --git a/tests/acceptance/07_packages/packages_command.cf b/tests/acceptance/07_packages/packages_command.cf new file mode 100644 index 0000000000..4bece4865f --- /dev/null +++ b/tests/acceptance/07_packages/packages_command.cf @@ -0,0 +1,92 @@ +####################################################### +# +# See ticket ENT-13536 for info +# +####################################################### + +body common control +{ + inputs => { "../default.cf.sub" }; + bundlesequence => { "G", "g", default("$(this.promise_filename)") }; + version => "1.0"; + cache_system_functions => "false"; +} + +body package_method mock +{ + package_changes => "individual"; + package_list_command => "$(g.pm) --list-installed"; + + package_list_name_regex => "^[^:]*"; + package_list_version_regex => ":(?<=:).*(?=:)"; + package_list_arch_regex => "[^:]\w+$"; + package_installed_regex => "^[^:]*"; + + package_add_command => "$(g.pm) --add "; + package_update_command => "$(g.pm) --update "; + package_delete_command => "$(g.pm) --delete "; + package_verify_command => "$(g.pm) --verify "; +} + +bundle common g +{ + classes: + "mpm_declared" not => strcmp(getenv("MOCK_PACKAGE_MANAGER", "65535"), ""); + + vars: + mpm_declared:: + "pm" string => getenv("MOCK_PACKAGE_MANAGER", "65535"); + + !mpm_declared:: + "pm" string => "$(G.mock_package_manager)"; +} + +####################################################### + +bundle agent init +{ + commands: + "$(g.pm) --clear-installed"; + "$(g.pm) --clear-available"; + "$(g.pm) --populate-available imagisoft:1.0i:teapot"; + + files: + "$(G.testfile)" + delete => tidy; +} + +####################################################### + +bundle agent test +{ + meta: + "test_skip_unsupported" string => "windows", + meta => { "ENT-10215" }; + + vars: + "name" string => "imagisoft"; + "version" string => "1.0i"; + "arch" string => "teapot"; + + packages: + "imagisoft; echo 'Hello CFEngine' > '$(G.testfile)';" + package_policy => "add", + package_method => mock; +} + +####################################################### + +bundle agent check +{ + reports: + "$(this.promise_filename) $(with)" + with => ifelse(fileexists("$(G.testfile)"), "FAIL", "Pass"); +} + +####################################################### + +bundle agent clean +{ + methods: + "init"; +}