From f4a35efec1b6a1e54d61cf1b8bfc83dd1d89eef7 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Fri, 5 Aug 2011 08:24:22 -0400 Subject: fixed security bugs with unescaped input to the shell --- src/lib/Server/Admin/Viz.py | 7 +++---- src/lib/Server/Plugins/Hg.py | 1 - src/lib/Server/Plugins/SSHbase.py | 3 +-- src/lib/Server/Plugins/SSLCA.py | 36 ++++++++++++++++-------------------- src/lib/Server/Plugins/Svn.py | 2 +- 5 files changed, 21 insertions(+), 28 deletions(-) diff --git a/src/lib/Server/Admin/Viz.py b/src/lib/Server/Admin/Viz.py index 3ab54e543..9b1e78821 100644 --- a/src/lib/Server/Admin/Viz.py +++ b/src/lib/Server/Admin/Viz.py @@ -86,11 +86,10 @@ class Viz(Bcfg2.Server.Admin.MetadataCore): 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: diff --git a/src/lib/Server/Plugins/Hg.py b/src/lib/Server/Plugins/Hg.py index 3f2864a1c..70e33ef1f 100644 --- a/src/lib/Server/Plugins/Hg.py +++ b/src/lib/Server/Plugins/Hg.py @@ -1,6 +1,5 @@ import os from mercurial import ui, hg -from subprocess import Popen, PIPE import Bcfg2.Server.Plugin # for debugging output only diff --git a/src/lib/Server/Plugins/SSHbase.py b/src/lib/Server/Plugins/SSHbase.py index b15275815..8cc4ef6f7 100644 --- a/src/lib/Server/Plugins/SSHbase.py +++ b/src/lib/Server/Plugins/SSHbase.py @@ -169,8 +169,7 @@ class SSHbase(Bcfg2.Server.Plugin.Plugin, 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) diff --git a/src/lib/Server/Plugins/SSLCA.py b/src/lib/Server/Plugins/SSLCA.py index baaa14ba9..a2ab3bf6b 100644 --- a/src/lib/Server/Plugins/SSLCA.py +++ b/src/lib/Server/Plugins/SSLCA.py @@ -3,6 +3,7 @@ import Bcfg2.Options import lxml.etree import posixpath import tempfile +import pipes import os from subprocess import Popen, PIPE, STDOUT # Compatibility import @@ -119,10 +120,10 @@ class SSLCA(Bcfg2.Server.Plugin.GroupSpool): 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): @@ -176,8 +177,8 @@ class SSLCA(Bcfg2.Server.Plugin.GroupSpool): """ 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 @@ -188,9 +189,11 @@ class SSLCA(Bcfg2.Server.Plugin.GroupSpool): """ 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 @@ -206,16 +209,11 @@ class SSLCA(Bcfg2.Server.Plugin.GroupSpool): 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) @@ -271,9 +269,7 @@ class SSLCA(Bcfg2.Server.Plugin.GroupSpool): 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 cb4ab649b..a127d0273 100644 --- a/src/lib/Server/Plugins/Svn.py +++ b/src/lib/Server/Plugins/Svn.py @@ -35,7 +35,7 @@ class Svn(Bcfg2.Server.Plugin.Plugin, """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] -- cgit v1.2.3-1-g7c22