CVE-2012-3366: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: The Trigger plugin in bcfg2 1.2.x before 1.2.3 allows remote attackers with root access to the client to execute arbitrary commands via shell metacharacters in the UUID field to the server process (bcfg2-server).
CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
Published: 2012-07-03T16:40Z
Repository: https://github.com/Bcfg2/bcfg2
Language: Python
Files changed: src/lib/Server/Plugins/Trigger.py
Commit date: 2012-06-12 09:20:10 -0400
Commit message: fixed major security flaw in Trigger plugin

Diff: diff --git a/src/lib/Server/Plugins/Trigger.py b/src/lib/Server/Plugins/Trigger.py
index eb3310a4..b173031f 100644
--- a/src/lib/Server/Plugins/Trigger.py
+++ b/src/lib/Server/Plugins/Trigger.py
@@ -1,19 +1,9 @@
 import os
+import pipes
 import Bcfg2.Server.Plugin
-
-
-def async_run(prog, args):
-    pid = os.fork()
-    if pid:
-        os.waitpid(pid, 0)
-    else:
-        dpid = os.fork()
-        if not dpid:
-            os.system(" ".join([prog] + args))
-        os._exit(0)
-
+from subprocess import Popen, PIPE
 
 class Trigger(Bcfg2.Server.Plugin.Plugin,
               Bcfg2.Server.Plugin.Statistics):
     """Trigger is a plugin that calls external scripts (on the server)."""
     name = 'Trigger'
@@ -28,17 +18,37 @@ class Trigger(Bcfg2.Server.Plugin.Plugin,
         except:
             self.logger.error("Trigger: spool directory %s does not exist; "
                               "unloading" % self.data)
             raise Bcfg2.Server.Plugin.PluginInitError
 
+    def async_run(self, args):
+        pid = os.fork()
+        if pid:
+            os.waitpid(pid, 0)
+        else:
+            dpid = os.fork()
+            if not dpid:
+                self.debug_log("Running %s" % " ".join(pipes.quote(a)
+                                                       for a in args))
+                proc = Popen(args, stdin=PIPE, stdout=PIPE, stderr=PIPE)
+                (out, err) = proc.communicate()
+                rv = proc.wait()
+                if rv != 0:
+                    self.logger.error("Trigger: Error running %s (%s): %s" %
+                                      (args[0], rv, err))
+                elif err:
+                    self.debug_log("Trigger: Error: %s" % err)
+            os._exit(0)
+
     def process_statistics(self, metadata, _):
         args = [metadata.hostname, '-p', metadata.profile, '-g',
                 ':'.join([g for g in metadata.groups])]
+        self.debug_log("running triggers")
         for notifier in os.listdir(self.data):
+            self.debug_log("running %s" % notifier)
             if ((notifier[-1] == '~') or
                 (notifier[:2] == '.#') or
                 (notifier[-4:] == '.swp') or
                 (notifier in ['SCCS', '.svn', '4913'])):
                 continue
-            npath = self.data + '/' + notifier
-            self.logger.debug("Running %s %s" % (npath, " ".join(args)))
-            async_run(npath, args)
+            npath = os.path.join(self.data, notifier)
+            self.async_run([npath] + args)

Vulnerable code: 

def async_run(prog, args):
    pid = os.fork()
    if pid:
        os.waitpid(pid, 0)
    else:
        dpid = os.fork()
        if not dpid:
            os.system(" ".join([prog] + args))
        os._exit(0)

            npath = self.data + '/' + notifier
            self.logger.debug("Running %s %s" % (npath, " ".join(args)))
            async_run(npath, args)
Fixed code: import pipes
from subprocess import Popen, PIPE
    def async_run(self, args):
        pid = os.fork()
        if pid:
            os.waitpid(pid, 0)
        else:
            dpid = os.fork()
            if not dpid:
                self.debug_log("Running %s" % " ".join(pipes.quote(a)
                                                       for a in args))
                proc = Popen(args, stdin=PIPE, stdout=PIPE, stderr=PIPE)
                (out, err) = proc.communicate()
                rv = proc.wait()
                if rv != 0:
                    self.logger.error("Trigger: Error running %s (%s): %s" %
                                      (args[0], rv, err))
                elif err:
                    self.debug_log("Trigger: Error: %s" % err)
            os._exit(0)

        self.debug_log("running triggers")
            self.debug_log("running %s" % notifier)
            npath = os.path.join(self.data, notifier)
            self.async_run([npath] + args)