On Fri, Sep 02, 2011 at 11:52:02PM +0200, Guido Günther wrote:
> On Thu, Aug 25, 2011 at 12:22:04PM +0200, Josselin Mouette wrote:
> > Package: system-config-printer
> > Version: 1.2.3-3
> > Severity: grave
> > Tags: security squeeze
> >
> > See https://bugzilla.redhat.com/show_bug.cgi?id=728348
> >
> > The pysmb.py module in system-config-printer is vulnerable to a remote
> > security vulnerability.
>
> I had a short look at the code now.
>
> >From what I see the version in squeeze uses smbc instead of invoking
> nmblookup/smbclient directly. There's a leftover nmblookup in
> troubleshoot/CheckPrinterSanity.py though but this only gets input from
> the troubleshoot dialog..
Lenny should be fixed by this patch. I'd appreciate any review.
Cheers,
-- Guido
>From ad21fcaa212f2ca546e2a2df04667b7d29b1674a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Guido=20G=C3=BCnther?= <a...@sigxcpu.org>
Date: Wed, 14 Sep 2011 13:37:18 +0200
Subject: [PATCH] Use subprocess.Popen with shell=False
instead of os.popen to avoid problems unsanitized input
---
pysmb.py | 73 +++++++++++++++++++++++++++++--------------------------------
1 files changed, 35 insertions(+), 38 deletions(-)
diff --git a/pysmb.py b/pysmb.py
index 2208678..b048283 100644
--- a/pysmb.py
+++ b/pysmb.py
@@ -30,6 +30,7 @@ import gobject
import gtk
import os
import pwd
+import subprocess
from debug import *
class AuthContext:
@@ -160,6 +161,15 @@ smbclient = "/usr/bin/smbclient"
wins = None
+def do_popen(cmd):
+ env = os.environ.copy()
+ env["LC_ALL"] = "C"
+
+ pipe = subprocess.Popen(cmd, shell=False, stdout=subprocess.PIPE,
+ stderr=subprocess.STDOUT, env=env)
+ return pipe.communicate()[0].splitlines(True)
+
+
def get_wins_server():
smbconf = "/etc/samba/smb.conf"
wsregex = re.compile("\s*wins\s*server.*",re.IGNORECASE)
@@ -201,35 +211,31 @@ def get_domain_list ():
ips = []
if wins:
- os.environ['WINS'] = wins
- str = 'LC_ALL=C %s -U "$WINS" -M -- - 2>&1' % (nmblookup)
+ cmd = [ nmblookup, '-U', wins, '-M', '--', '-' ]
else:
- str = "LC_ALL=C %s -M -- - 2>&1" % (nmblookup)
- for l in os.popen (str, 'r').readlines ():
+ cmd = [ nmblookup, '-M', '--', '-' ]
+
+ for l in do_popen(cmd):
l = l.splitlines()[0]
if l.endswith("<01>"):
ips.append (l.split(" ")[0])
if len (ips) <= 0:
if wins:
- os.environ['WINS'] = wins
- str = 'LC_ALL=C %s -U "$WINS" "*" 2>&1' % (nmblookup)
+ cmd = [ nmblookup, '-U', wins, '*' ]
else:
- str = "LC_ALL=C %s '*' 2>&1" % (nmblookup)
- for l in os.popen (str, 'r').readlines ():
+ cmd = [ nmblookup, '*' ]
+ for l in do_popen(cmd):
l = l.splitlines()[0]
ips.append (l.split(" ")[0])
for ip in ips:
dom = None
dict = { 'IP': ip }
- os.environ["IP"] = ip
if wins:
- os.environ["WINS"] = wins
- str = 'LC_ALL=C %s -U "$WINS" -A "$IP"' % (nmblookup)
+ cmd = [ 'nmblookup', '-U', wins, '-A', ip ]
else:
- str = 'LC_ALL=C %s -A "$IP"' % (nmblookup)
- str += " 2>&1"
- for line in os.popen(str, 'r').readlines():
+ cmd = [ 'nmblookup', '-A', ip ]
+ for line in do_popen(cmd):
line = line.splitlines()[0]
if (line.find(" <00> ") != -1) and (line.find("<GROUP>") != -1):
dom = line.split(" ")[0]
@@ -252,9 +258,8 @@ def get_host_list(dmbip):
serverregex = re.compile("\s*Server\s*Comment")
domainregex = re.compile("\s*Workgroup\s*Master")
commentregex = re.compile("(\s*-+)+")
- os.environ["DMBIP"] = dmbip
- str = 'LC_ALL=C %s -N -L "//$DMBIP" 2>/dev/null' % (smbclient)
- for l in os.popen (str, 'r').readlines ():
+ cmd = [ smbclient, '-N', '-L', '//%s' % dmbip ]
+ for l in do_popen(cmd):
l = l.splitlines()[0]
if serverregex.match(l):
@@ -295,10 +300,10 @@ def get_host_list_from_domain (domain):
global wins
ips = []
if wins:
- str = "LC_ALL=C %s -U %s -R '%s' 2>&1" % (nmblookup, wins, domain)
+ cmd = [ nmblookup, '-U', wins, '-R', domain ]
else:
- str = "LC_ALL=C %s -R '%s' 2>&1" % (nmblookup, domain)
- for l in os.popen (str, 'r').readlines ():
+ cmd = [ nmblookup, '-R', domain ]
+ for l in do_popen(cmd):
l = l.splitlines()[0]
if l.endswith("<00>"):
ips.append (l.split(" ")[0])
@@ -306,14 +311,11 @@ def get_host_list_from_domain (domain):
for ip in ips:
name = None
dict = { 'IP': ip }
- os.environ["IP"] = ip
if wins:
- os.environ["WINS"] = wins
- str = 'LC_ALL=C %s -U "$WINS" -A "$IP"' % (nmblookup)
+ cmd = [ nmblookup, '-U', wins, '-A', ip ]
else:
- str = 'LC_ALL=C %s -A "$IP"' % (nmblookup)
- str += " 2>&1"
- for line in os.popen(str, 'r').readlines():
+ cmd = [ nmblookup, '-A', ip ]
+ for line in do_popen(cmd):
line = line.splitlines()[0]
if (line.find(" <00> ") != -1) and (line.find("<GROUP>") == -1):
name = line.split(" ")[0]
@@ -331,13 +333,11 @@ def get_host_info (smbname):
"""Given an SMB name, returns a host dict for it."""
dict = { 'NAME': smbname, 'IP': '', 'GROUP': '' }
global wins
- os.environ["SMBNAME"] = smbname
if wins:
- os.environ["WINS"] = wins
- str = 'LC_ALL=C %s -U "$WINS" -S "$SMBNAME" 2>&1' % (nmblookup)
+ cmd = [ nmblookup, '-U', wins, '-S', smbname ]
else:
- str = 'LC_ALL=C %s -S "$SMBNAME" 2>&1' % (nmblookup)
- for l in os.popen (str, 'r').readlines ():
+ cmd = [ nmblookup, '-S', smbname ]
+ for l in do_popen(cmd):
l = l.strip ()
if l.endswith ("<00>"):
dict['IP'] = l.split (" ")[0]
@@ -362,20 +362,17 @@ def get_printer_list (host):
if not os.access (smbclient, os.X_OK):
return printers
- os.environ["NAME"] = host['NAME']
- str = 'LC_ALL=C %s -N -L "$NAME" 2>&1' % (smbclient)
+ cmd = [ smbclient, '-N', '-L', host['NAME'] ]
if host.has_key ('IP'):
- os.environ["IP"] = host['IP']
- str += ' -I "$IP"'
+ cmd += [ '-I', host['IP'] ]
if host.has_key ('GROUP'):
- os.environ["GROUP"] = host['GROUP']
- str += ' -W "$GROUP"'
+ cmd += [ '-W', host['GROUP'] ]
section = 0
typepos = 0
commentpos = 0
- for l in os.popen (str, 'r'):
+ for l in do_popen(cmd):
l = l.strip ()
if l == "":
continue
--
1.7.5.4
