Added packages promiser sanitation

Ticket: ENT-13535
Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>

Author: larsewi

cf-agent/verify_packages.c

@@ -54,6 +54,7 @@
 #include <csv_writer.h>
 #include <cf-agent-enterprise-stubs.h>
 #include <cf-windows-functions.h>
+#include <cf3.defs.h>
 
 /* 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
 

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";
+}