Use sequence based command invocation more; avoids shell escaping/whitespace issues eg. like https://bugzilla.redhat.com/206383

git-svn-id: svn+ssh://rpmlint.zarb.org/home/projects/rpmlint/svn/trunk@1275 9bc8b190-ac0f-0410-8968-dc7d1f502856

BinariesCheck.py

@@ -11,7 +11,6 @@ from Filter import *
 import AbstractCheck
 import rpm
 import re
-import commands
 import string
 import sys
 import Config
@@ -45,8 +44,10 @@ class BinaryInfo:
 
         is_debug=BinaryInfo.debug_file_regex.search(path)
 
-        topt=not is_debug and '-T' or ''
-        res=commands.getstatusoutput('LC_ALL=C objdump --headers --private-headers %s %s' % (topt, path))
+        cmd = ['env', 'LC_ALL=C', 'objdump', '--headers', '--private-headers']
+        if not is_debug: cmd.append('-T')
+        cmd.append(path)
+        res = Pkg.getstatusoutput(cmd)
         if not res[0]:
             for l in string.split(res[1], '\n'):
                 needed=BinaryInfo.needed_regex.search(l)
@@ -76,7 +77,7 @@ class BinaryInfo:
         # skip debuginfo: https://bugzilla.redhat.com/190599
         if not is_debug and isinstance(pkg, Pkg.InstalledPkg):
             # We could do this with objdump, but it's _much_ simpler with ldd.
-            res=commands.getstatusoutput('LC_ALL=C ldd -d -r %s' % path)
+            res = Pkg.getstatusoutput(('env', 'LC_ALL=C', 'ldd', '-d', '-r', path))
             if not res[0]:
                 for l in string.split(res[1], '\n'):
                     undef=BinaryInfo.undef_regex.search(l)

MenuCheck.py

@@ -9,9 +9,9 @@
 
 from Filter import *
 import AbstractCheck
+import Pkg
 import rpm
 import re
-import commands
 import string
 import stat
 
@@ -220,9 +220,10 @@ class MenuCheck(AbstractCheck.AbstractCheck):
 
             for f in menus:
                 # remove comments and handle cpp continuation lines
-                command_str='/lib/cpp %s%s 2>/dev/null| grep ^\?' % (directory, f)
-                cmd=commands.getoutput(command_str)
+                sts, cmd = Pkg.getstatusoutput(('/lib/cpp', directory + f), 1)
+                                                  
                 for line in string.split(cmd, '\n'):
+                    if not line.startswith('?'): continue
                     res=package_regex.search(line)
                     if res:
                         package=res.group(1)

Pkg.py

@@ -13,6 +13,7 @@ import rpm
 import os.path
 import stat
 import commands
+import popen2
 import re
 import string
 import types
@@ -78,6 +79,20 @@ def substitute_shell_vars(val, script):
     else:
         return val
 
+def getstatusoutput(cmd, stdoutonly=0):
+    '''A version of commands.getstatusoutput() which can take cmd as a
+       sequence, thus making it potentially more secure.  See popen2.'''
+    if stdoutonly:
+        proc = popen2.Popen3(cmd)
+    else:
+        proc = popen2.Popen4(cmd)
+    proc.tochild.close()
+    text = proc.fromchild.read()
+    sts = proc.wait()
+    if sts is None: sts = 0
+    if text[-1:] == '\n': text = text[:-1]
+    return sts, text
+
 bz2_regex=re.compile('\.t?bz2?$')
 
 # TODO: is_utf8* could probably be implemented natively without iconv...
@@ -85,7 +100,8 @@ bz2_regex=re.compile('\.t?bz2?$')
 def is_utf8(fname):
     cat='gzip -dcf'
     if bz2_regex.search(fname): cat='bzip2 -dcf'
-    cmd = commands.getstatusoutput('%s %s | iconv -f utf-8 -t utf-8 -o /dev/null' % (cat, fname))
+    # TODO: better shell escaping or sequence based command invocation
+    cmd = commands.getstatusoutput('%s "%s" | iconv -f utf-8 -t utf-8 -o /dev/null' % (cat, fname))
     return not cmd[0]
 
 def is_utf8_str(s):
@@ -167,19 +183,23 @@ class Pkg:
         else:
             self.dirname = '%s/%s.%d' % (self.dirname, os.path.basename(self.filename), os.getpid())
             os.mkdir(self.dirname)
-            command_str='rpm2cpio %s | (cd %s; cpio -id); chmod -R +rX %s' % (self.filename, self.dirname, self.dirname)
+            # TODO: better shell escaping or sequence based command invocation
+            command_str='rpm2cpio "%s" | (cd "%s"; cpio -id); chmod -R +rX "%s"' % (self.filename, self.dirname, self.dirname)
             cmd=commands.getstatusoutput(command_str)
             self.extracted=1
             return cmd
 
     def checkSignature(self):
-        return commands.getstatusoutput('LC_ALL=C rpm -K ' + self.filename)
+        return getstatusoutput(('env', 'LC_ALL=C', 'rpm', '-K', self.filename))
 
     # return the array of info returned by the file command on each file
     def getFilesInfo(self):
         if self.file_info == None:
             self.file_info=[]
-            lines=commands.getoutput('cd %s; find . -type f -print0 | LC_ALL=C xargs -0r file' % (self.dirName()))
+            olddir = os.getcwd()
+            os.chdir(self.dirName())
+            lines = commands.getoutput('find . -type f -print0 | LC_ALL=C xargs -0r file')
+            os.chdir(olddir)
             lines=string.split(lines, '\n')
             for l in lines:
                 #print l
@@ -191,8 +211,8 @@ class Pkg:
 
     # remove the extracted files from the package
     def cleanup(self):
-        if self.extracted:
-            commands.getstatusoutput('rm -rf ' + self.dirname)
+        if self.extracted and self.dirname:
+            getstatusoutput(('rm', '-rf', self.dirname))
 
     def grep(self, regex, filename):
         """Grep regex from a file, return matching line numbers."""
@@ -455,10 +475,9 @@ class InstalledPkg(Pkg):
     def getFilesInfo(self):
         if self.file_info == None:
             self.file_info=[]
-            cmd='LC_ALL=C file'
-            for f in self.files().keys():
-                cmd=cmd + ' ' + f
-            lines=commands.getoutput(cmd)
+            cmd = ['env', 'LC_ALL=C', 'file']
+            cmd.extend(self.files().keys())
+            sts, lines = getstatusoutput(cmd)
             #print lines
             lines=string.split(lines, '\n')
             for l in lines:

PostCheck.py

@@ -13,7 +13,7 @@ import AbstractCheck
 import rpm
 import re
 import os
-import commands
+import Pkg
 import string
 import types
 
@@ -66,7 +66,7 @@ def incorrect_shell_script(prog, shellscript):
     f = open(tmpfile, 'w')
     f.write(shellscript)
     f.close()
-    ret=commands.getstatusoutput('%s -n %s' % (prog, tmpfile))
+    ret = Pkg.getstatusoutput((prog, '-n', tmpfile))
     os.remove(tmpfile)
     return ret[0]
 
@@ -80,7 +80,7 @@ def incorrect_perl_script(prog, perlscript):
     f = open(tmpfile, 'w')
     f.write(perlscript)
     f.close()
-    ret=commands.getstatusoutput('%s -wc %s' % (prog, tmpfile))
+    ret = Pkg.getstatusoutput((prog, '-wc', tmpfile))
     os.remove(tmpfile)
     return ret[0]