Harden security paths, shell invocation, and password hashing.

Fix private archive path traversal, replace os.popen/os.system with
subprocess, and upgrade site passwords, auth cookies, CSRF tokens, and
subscribe form tokens to stronger algorithms while keeping legacy
verification for in-place upgrades.

Author: jaredmauch

Mailman/Archiver/Archiver.py

@@ -176,12 +176,15 @@ def ExternalArchive(self, ar, txt):
                       'hostname': self.host_name,
                       })
         cmd = ar % d
-        extarch = os.popen(cmd, 'w')
-        extarch.write(txt)
-        status = extarch.close()
-        if status:
-            syslog('error', 'external archiver non-zero exit status: %d\n',
-                   (status & 0xff00) >> 8)
+        try:
+            result = Utils.run_command(cmd, input_data=txt)
+        except (ValueError, OSError) as e:
+            syslog('error', 'external archiver failed to run: %s', e)
+            return
+        if result.returncode:
+            stderr = result.stderr.decode('utf-8', 'replace') if result.stderr else ''
+            syslog('error', 'external archiver non-zero exit status: %d\n%s',
+                   result.returncode, stderr)
 
     #
     # archiving in real time  this is called from list.post(msg)

Mailman/CSRFcheck.py

@@ -24,7 +24,7 @@
 
 from Mailman import mm_cfg
 from Mailman.Logging.Syslog import syslog
-from Mailman.Utils import UnobscureEmail, sha_new
+from Mailman.Utils import UnobscureEmail, auth_context_mac, verify_auth_context_mac
 
 keydict = {
     'user':      mm_cfg.AuthUser,
@@ -50,8 +50,7 @@ def csrf_token(mlist, contexts, user=None):
     else:
         return None     # not authenticated
     issued = int(time.time())
-    needs_hash = (secret + repr(issued)).encode('utf-8')
-    mac = sha_new(needs_hash).hexdigest()
+    mac = auth_context_mac(secret, issued)
     keymac = '%s:%s' % (key, mac)
     token = marshal.dumps((issued, keymac)).hex()
 
@@ -96,9 +95,7 @@ def csrf_check(mlist, token, cgi_user=None):
         context = keydict.get(key)
         key, secret = mlist.AuthContextInfo(context, user)
         assert key
-        secret = secret + repr(issued)
-        mac = sha_new(secret.encode()).hexdigest()
-        if (mac == received_mac 
+        if (verify_auth_context_mac(secret, issued, received_mac)
             and 0 < time.time() - issued < mm_cfg.FORM_LIFETIME):
             return True
         return False

Mailman/Cgi/listinfo.py

@@ -233,7 +233,7 @@ def list_listinfo(mlist, lang):
             # just to have something to include in the hash below
             captcha_idx = ''
         secret = mm_cfg.SUBSCRIBE_FORM_SECRET + ":" + now + ":" + captcha_idx + ":" + mlist.internal_name() + ":" + remote
-        hash_secret = Utils.sha_new(secret.encode('utf-8')).hexdigest()
+        hash_secret = Utils.subscribe_form_token_digest(secret)
         # fill form
         replacements['<mm-subscribe-form-start>'] += (
                 '<input type="hidden" name="sub_form_token"'

Mailman/Cgi/private.py

@@ -39,15 +39,6 @@
 SLASH = '/'
 
 
-
-def true_path(path):
-    "Ensure that the path is safe by removing .."
-    # Workaround for path traverse vulnerability.  Unsuccessful attempts will
-    # be logged in logs/error.
-    parts = [x for x in path.split(SLASH) if x not in ('.', '..')]
-    return SLASH.join(parts)[1:]
-
-
 
 def guess_type(url, strict):
     if hasattr(mimetypes, 'common_types'):
@@ -68,17 +59,28 @@ def main():
         return
 
     path = os.environ.get('PATH_INFO')
-    tpath = true_path(path)
-    if tpath != path[1:]:
+    if not path:
+        doc.SetTitle(_("Private Archive Error"))
+        doc.AddItem(Header(3, _("No archive path specified.")))
+        print(doc.Format())
+        return
+    if any(part in ('.', '..') for part in path.split(SLASH)):
         msg = _('Private archive - "./" and "../" not allowed in URL.')
         doc.SetTitle(msg)
         doc.AddItem(Header(2, msg))
         print(doc.Format())
         syslog('mischief', 'Private archive hostile path: %s', path)
         return
-    # BAW: This needs to be converted to the Site module abstraction
-    true_filename = os.path.join(
-        mm_cfg.PRIVATE_ARCHIVE_FILE_DIR, tpath)
+    try:
+        true_filename = Utils.safe_path_under(
+            mm_cfg.PRIVATE_ARCHIVE_FILE_DIR, path)
+    except ValueError:
+        msg = _('Private archive - invalid path.')
+        doc.SetTitle(msg)
+        doc.AddItem(Header(2, msg))
+        print(doc.Format())
+        syslog('mischief', 'Private archive hostile path: %s', path)
+        return
 
     listname = parts[0].lower()
     mboxfile = ''

Mailman/Cgi/subscribe.py

@@ -144,18 +144,17 @@ def process_form(mlist, doc, cgidata, lang):
                 'response': cgidata.getvalue('g-recaptcha-response', ''),
                 'remoteip': remote})
         request_data = request_data.encode('utf-8')
-        request = urllib.request.Request(
-            url = 'https://www.google.com/recaptcha/api/siteverify',
-            data = request_data)
         try:
-            httpresp = urllib.request.urlopen(request)
+            httpresp = Utils.safe_urlopen(
+                'https://www.google.com/recaptcha/api/siteverify',
+                data=request_data)
             captcha_response = json.load(httpresp)
             httpresp.close()
             if not captcha_response['success']:
                 e_codes = COMMASPACE.join(captcha_response['error-codes'])
                 results.append(_(f'reCAPTCHA validation failed: {e_codes}'))
-        except urllib.error.URLError as e:
-            e_reason = e.reason
+        except (urllib.error.URLError, ValueError) as e:
+            e_reason = getattr(e, 'reason', e)
             results.append(_(f'reCAPTCHA could not be validated: {e_reason}'))
 
     # Are we checking the hidden data?
@@ -177,8 +176,8 @@ def process_form(mlist, doc, cgidata, lang):
         except ValueError:
             ftime = fcaptcha_idx = fhash = ''
             then = 0
-        needs_hashing = (mm_cfg.SUBSCRIBE_FORM_SECRET + ":" + ftime + ":" + fcaptcha_idx + ":" + mlist.internal_name() + ":" + remote1).encode('utf-8')
-        token = Utils.sha_new(needs_hashing).hexdigest()
+        needs_hashing = (mm_cfg.SUBSCRIBE_FORM_SECRET + ":" + ftime + ":" + fcaptcha_idx + ":" + mlist.internal_name() + ":" + remote1)
+        token = Utils.subscribe_form_token_digest(needs_hashing)
         if ftime and now - then > mm_cfg.FORM_LIFETIME:
             results.append(_('The form is too old.  Please GET it again.'))
         if ftime and now - then < mm_cfg.SUBSCRIBE_FORM_MIN_TIME:

Mailman/Handlers/Scrubber.py

@@ -522,13 +522,16 @@ def save_attachment(mlist, msg, dir, filter_html=True):
             fp.write(decodedpayload)
             fp.close()
             cmd = mm_cfg.ARCHIVE_HTML_SANITIZER % {'filename' : tmppath}
-            progfp = os.popen(cmd, 'r')
-            decodedpayload = progfp.read()
-            status = progfp.close()
-            if status:
-                syslog('error',
-                       'HTML sanitizer exited with non-zero status: %s',
-                       status)
+            try:
+                result = Utils.run_command(cmd, capture_output=True)
+            except (ValueError, OSError) as e:
+                syslog('error', 'HTML sanitizer failed to run: %s', e)
+            else:
+                decodedpayload = result.stdout.decode('utf-8', 'replace')
+                if result.returncode:
+                    syslog('error',
+                           'HTML sanitizer exited with non-zero status: %s',
+                           result.returncode)
         finally:
             os.unlink(tmppath)
         # BAW: Since we've now sanitized the document, it should be plain

Mailman/LockFile.py

@@ -89,8 +89,7 @@ def _get_logfile():
         except ImportError:
             # not running inside Mailman
             import tempfile
-            dir = os.path.split(tempfile.mktemp())[0]
-            path = os.path.join(dir, 'LockFile.log')
+            path = os.path.join(tempfile.gettempdir(), 'LockFile.log')
             # open in line-buffered mode
             class SimpleUserFile(object):
                 def __init__(self, path):

Mailman/MTA/Postfix.py

@@ -64,20 +64,28 @@ def fixom(file):
             os.chown(dbfile, uid, gid)
     msg = 'command failed: %s (status: %s, %s)'
     acmd = mm_cfg.POSTFIX_ALIAS_CMD + ' ' + ALIASFILE
-    status = (os.system(acmd) >> 8) & 0xff
-    if status:
-        errstr = os.strerror(status)
-        syslog('error', msg, acmd, status, errstr)
-        raise RuntimeError(msg % (acmd, status, errstr))
+    try:
+        aresult = Utils.run_command(acmd)
+    except (ValueError, OSError) as e:
+        syslog('error', msg, acmd, -1, e)
+        raise RuntimeError(msg % (acmd, -1, e))
+    if aresult.returncode:
+        errstr = os.strerror(aresult.returncode)
+        syslog('error', msg, acmd, aresult.returncode, errstr)
+        raise RuntimeError(msg % (acmd, aresult.returncode, errstr))
     # Fix owner and mode of .db if needed.
     fixom(ALIASFILE)
     if os.path.exists(VIRTFILE):
         vcmd = mm_cfg.POSTFIX_MAP_CMD + ' ' + VIRTFILE
-        status = (os.system(vcmd) >> 8) & 0xff
-        if status:
-            errstr = os.strerror(status)
-            syslog('error', msg, vcmd, status, errstr)
-            raise RuntimeError(msg % (vcmd, status, errstr))
+        try:
+            vresult = Utils.run_command(vcmd)
+        except (ValueError, OSError) as e:
+            syslog('error', msg, vcmd, -1, e)
+            raise RuntimeError(msg % (vcmd, -1, e))
+        if vresult.returncode:
+            errstr = os.strerror(vresult.returncode)
+            syslog('error', msg, vcmd, vresult.returncode, errstr)
+            raise RuntimeError(msg % (vcmd, vresult.returncode, errstr))
         # Fix owner and mode of .db if needed.
         fixom(VIRTFILE)
 

Mailman/SecurityManager.py

@@ -63,7 +63,8 @@
 from Mailman import Utils
 from Mailman import Errors
 from Mailman.Logging.Syslog import syslog
-from Mailman.Utils import sha_new, hash_password, verify_password
+from Mailman.Utils import hash_password, verify_password
+from Mailman.Utils import auth_context_mac, verify_auth_context_mac
 
 
 class SecurityManager(object):
@@ -317,9 +318,7 @@ def MakeCookie(self, authcontext, user=None):
             raise ValueError
         # Timestamp
         issued = int(time.time())
-        # Get a digest of the secret, plus other information.
-        needs_hashing = (secret + repr(issued)).encode('utf-8')
-        mac = sha_new(needs_hashing).hexdigest()
+        mac = auth_context_mac(secret, issued)
         # Create the cookie object.
         c = http.cookies.SimpleCookie()
         c[key] = binascii.hexlify(marshal.dumps((issued, mac))).decode()
@@ -427,9 +426,7 @@ def __checkone(self, c, authcontext, user):
             return False
         # Calculate what the mac ought to be based on the cookie's timestamp
         # and the shared secret.
-        needs_hashing = (secret + repr(issued)).encode('utf-8')
-        mac = sha_new(needs_hashing).hexdigest()
-        if mac != received_mac:
+        if not verify_auth_context_mac(secret, issued, received_mac):
             return False
         # Authenticated!
         # Refresh the cookie