CVE-2011-3211: Improper Input Validation (CWE-20)
value: The server in Bcfg2 1.1.2 and earlier, and 1.2 prerelease, allows remote attackers to execute arbitrary commands via shell metacharacters in data received from a client.
CWE description: The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
Published: 2011-09-16T12:35Z
Repository: https://github.com/solj/bcfg2
Language: Python
Files changed: src/lib/Server/Admin/Viz.py, src/lib/Server/Plugins/Hg.py, src/lib/Server/Plugins/SSHbase.py, src/lib/Server/Plugins/SSLCA.py, src/lib/Server/Plugins/Svn.py
Commit date: 2011-08-05 08:24:22 -0400
Commit message: fixed security bugs with unescaped input to the shell

Diff: diff --git a/src/lib/Server/Admin/Viz.py b/src/lib/Server/Admin/Viz.py
index 3ab54e54..9b1e7882 100644
--- a/src/lib/Server/Admin/Viz.py
+++ b/src/lib/Server/Admin/Viz.py
@@ -84,15 +84,14 @@ class Viz(Bcfg2.Server.Admin.MetadataCore):
         if output:
             format = output.split('.')[-1]
         else:
             format = 'png'
 
-        cmd = "dot -T%s" % (format)
+        cmd = ["dot", "-T", format]
         if output:
-            cmd += " -o %s" % output
-        dotpipe = Popen(cmd, shell=True, stdin=PIPE,
-                        stdout=PIPE, close_fds=True)
+            cmd.extend(["-o", output])
+        dotpipe = Popen(cmd, stdin=PIPE, stdout=PIPE, close_fds=True)
         try:
             dotpipe.stdin.write("digraph groups {\n")
         except:
             print("write to dot process failed. Is graphviz installed?")
             raise SystemExit(1)
diff --git a/src/lib/Server/Plugins/Hg.py b/src/lib/Server/Plugins/Hg.py
index 3f2864a1..70e33ef1 100644
--- a/src/lib/Server/Plugins/Hg.py
+++ b/src/lib/Server/Plugins/Hg.py
@@ -1,8 +1,7 @@
 import os
 from mercurial import ui, hg
-from subprocess import Popen, PIPE
 import Bcfg2.Server.Plugin
 
 # for debugging output only
 import logging
 logger = logging.getLogger('Bcfg2.Plugins.Mercurial')
diff --git a/src/lib/Server/Plugins/SSHbase.py b/src/lib/Server/Plugins/SSHbase.py
index b1527581..8cc4ef6f 100644
--- a/src/lib/Server/Plugins/SSHbase.py
+++ b/src/lib/Server/Plugins/SSHbase.py
@@ -167,12 +167,11 @@ class SSHbase(Bcfg2.Server.Plugin.Plugin,
             try:
                 ipaddr = socket.gethostbyname(client)
                 self.ipcache[client] = (ipaddr, client)
                 return (ipaddr, client)
             except socket.gaierror:
-                cmd = "getent hosts %s" % client
-                ipaddr = Popen(cmd, shell=True, \
+                ipaddr = Popen(["getent", "hosts", client],
                                stdout=PIPE).stdout.read().strip().split()
                 if ipaddr:
                     self.ipcache[client] = (ipaddr, client)
                     return (ipaddr, client)
                 self.ipcache[client] = False
diff --git a/src/lib/Server/Plugins/SSLCA.py b/src/lib/Server/Plugins/SSLCA.py
index baaa14ba..a2ab3bf6 100644
--- a/src/lib/Server/Plugins/SSLCA.py
+++ b/src/lib/Server/Plugins/SSLCA.py
@@ -1,10 +1,11 @@
 import Bcfg2.Server.Plugin
 import Bcfg2.Options
 import lxml.etree
 import posixpath
 import tempfile
+import pipes
 import os
 from subprocess import Popen, PIPE, STDOUT
 # Compatibility import
 from Bcfg2.Bcfg2Py3k import ConfigParser
 
@@ -117,14 +118,14 @@ class SSLCA(Bcfg2.Server.Plugin.GroupSpool):
         generates a new key according the the specification
         """
         type = self.key_specs[entry.get('name')]['type']
         bits = self.key_specs[entry.get('name')]['bits']
         if type == 'rsa':
-            cmd = "openssl genrsa %s " % bits
+            cmd = ["openssl", "genrsa", bits]
         elif type == 'dsa':
-            cmd = "openssl dsaparam -noout -genkey %s" % bits
-        key = Popen(cmd, shell=True, stdout=PIPE).stdout.read()
+            cmd = ["openssl", "dsaparam", "-noout", "-genkey", bits]
+        key = Popen(cmd, stdout=PIPE).stdout.read()
         return key
 
     def get_cert(self, entry, metadata):
         """
         either grabs a prexisting cert hostfile, or triggers the generation
@@ -174,25 +175,27 @@ class SSLCA(Bcfg2.Server.Plugin.GroupSpool):
         check that a certificate validates against the ca cert,
         and that it has not expired.
         """
         chaincert = self.CAs[self.cert_specs[entry.get('name')]['ca']].get('chaincert')
         cert = self.data + filename
-        cmd = "openssl verify -CAfile %s %s" % (chaincert, cert)
-        res = Popen(cmd, shell=True, stdout=PIPE, stderr=STDOUT).stdout.read()
+        res = Popen(["openssl", "verify", "-CAfile", chaincert, cert],
+                    stdout=PIPE, stderr=STDOUT).stdout.read()
         if res == cert + ": OK\n":
             return True
         return False
 
     def verify_cert_against_key(self, filename, key_filename):
         """
         check that a certificate validates against its private key.
         """
         cert = self.data + filename
         key = self.data + key_filename
-        cmd = "openssl x509 -noout -modulus -in %s | openssl md5" % cert
+        cmd = ("openssl x509 -noout -modulus -in %s | openssl md5" %
+               pipes.quote(cert))
         cert_md5 = Popen(cmd, shell=True, stdout=PIPE, stderr=STDOUT).stdout.read()
-        cmd = "openssl rsa -noout -modulus -in %s | openssl md5" % key
+        cmd = ("openssl rsa -noout -modulus -in %s | openssl md5" %
+               pipes.quote(key))
         key_md5 = Popen(cmd, shell=True, stdout=PIPE, stderr=STDOUT).stdout.read()
         if cert_md5 == key_md5:
             return True
         return False
 
@@ -204,20 +207,15 @@ class SSLCA(Bcfg2.Server.Plugin.GroupSpool):
         req = self.build_request(key_filename, req_config, entry)
         ca = self.cert_specs[entry.get('name')]['ca']
         ca_config = self.CAs[ca]['config']
         days = self.cert_specs[entry.get('name')]['days']
         passphrase = self.CAs[ca].get('passphrase')
+        cmd = ["openssl", "ca", "-config", ca_config, "-in", req,
+               "-days", days, "-batch"]
         if passphrase:
-            cmd = "openssl ca -config %s -in %s -days %s -batch -passin pass:%s" % (ca_config,
-                                                                                    req,
-                                                                                    days,
-                                                                                    passphrase)
-        else:
-            cmd = "openssl ca -config %s -in %s -days %s -batch" % (ca_config,
-                                                                    req,
-                                                                    days)
-        cert = Popen(cmd, shell=True, stdout=PIPE).stdout.read()
+            cmd.extend(["-passin", "pass:%s" % passphrase])
+        cert = Popen(cmd, stdout=PIPE).stdout.read()
         try:
             os.unlink(req_config)
             os.unlink(req)
         except OSError:
             self.logger.error("Failed to unlink temporary files")
@@ -269,11 +267,9 @@ class SSLCA(Bcfg2.Server.Plugin.GroupSpool):
         creates the certificate request
         """
         req = tempfile.mkstemp()[1]
         days = self.cert_specs[entry.get('name')]['days']
         key = self.data + key_filename
-        cmd = "openssl req -new -config %s -days %s -key %s -text -out %s" % (req_config,
-                                                                              days,
-                                                                              key,
-                                                                              req)
+        cmd = ["openssl", "req", "-new", "-config", req_config,
+               "-days", days, "-key", key, "-text", "-out", req]
         res = Popen(cmd, shell=True, stdout=PIPE).stdout.read()
         return req
diff --git a/src/lib/Server/Plugins/Svn.py b/src/lib/Server/Plugins/Svn.py
index cb4ab649..a127d027 100644
--- a/src/lib/Server/Plugins/Svn.py
+++ b/src/lib/Server/Plugins/Svn.py
@@ -33,11 +33,11 @@ class Svn(Bcfg2.Server.Plugin.Plugin,
 
     def get_revision(self):
         """Read svn revision information for the Bcfg2 repository."""
         try:
             data = Popen(("env LC_ALL=C svn info %s" %
-                         (self.datastore)), shell=True,
+                         pipes.quote(self.datastore)), shell=True,
                          stdout=PIPE).communicate()[0].split('\n')
             return [line.split(': ')[1] for line in data \
                     if line[:9] == 'Revision:'][-1]
         except IndexError:
             logger.error("Failed to read svn info; disabling svn support")

Vulnerable code:         cmd = "dot -T%s" % (format)
            cmd += " -o %s" % output
        dotpipe = Popen(cmd, shell=True, stdin=PIPE,
                        stdout=PIPE, close_fds=True)
from subprocess import Popen, PIPE
                cmd = "getent hosts %s" % client
                ipaddr = Popen(cmd, shell=True, \
            cmd = "openssl genrsa %s " % bits
            cmd = "openssl dsaparam -noout -genkey %s" % bits
        key = Popen(cmd, shell=True, stdout=PIPE).stdout.read()
        cmd = "openssl verify -CAfile %s %s" % (chaincert, cert)
        res = Popen(cmd, shell=True, stdout=PIPE, stderr=STDOUT).stdout.read()
        cmd = "openssl x509 -noout -modulus -in %s | openssl md5" % cert
        cmd = "openssl rsa -noout -modulus -in %s | openssl md5" % key
            cmd = "openssl ca -config %s -in %s -days %s -batch -passin pass:%s" % (ca_config,
                                                                                    req,
                                                                                    days,
                                                                                    passphrase)
        else:
            cmd = "openssl ca -config %s -in %s -days %s -batch" % (ca_config,
                                                                    req,
                                                                    days)
        cert = Popen(cmd, shell=True, stdout=PIPE).stdout.read()
        cmd = "openssl req -new -config %s -days %s -key %s -text -out %s" % (req_config,
                                                                              days,
                                                                              key,
                                                                              req)
                         (self.datastore)), shell=True,
Fixed code:         cmd = ["dot", "-T", format]
            cmd.extend(["-o", output])
        dotpipe = Popen(cmd, stdin=PIPE, stdout=PIPE, close_fds=True)
                ipaddr = Popen(["getent", "hosts", client],
import pipes
            cmd = ["openssl", "genrsa", bits]
            cmd = ["openssl", "dsaparam", "-noout", "-genkey", bits]
        key = Popen(cmd, stdout=PIPE).stdout.read()
        res = Popen(["openssl", "verify", "-CAfile", chaincert, cert],
                    stdout=PIPE, stderr=STDOUT).stdout.read()
        cmd = ("openssl x509 -noout -modulus -in %s | openssl md5" %
               pipes.quote(cert))
        cmd = ("openssl rsa -noout -modulus -in %s | openssl md5" %
               pipes.quote(key))
        cmd = ["openssl", "ca", "-config", ca_config, "-in", req,
               "-days", days, "-batch"]
            cmd.extend(["-passin", "pass:%s" % passphrase])
        cert = Popen(cmd, stdout=PIPE).stdout.read()
        cmd = ["openssl", "req", "-new", "-config", req_config,
               "-days", days, "-key", key, "-text", "-out", req]
                         pipes.quote(self.datastore)), shell=True,