Blob Blame History Raw
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <encukou@gmail.com>
Date: Fri, 3 Jun 2022 11:43:35 +0200
Subject: [PATCH] 00382-cve-2015-20107.patch

00382 #
Make mailcap refuse to match unsafe filenames/types/params (GH-91993)

Upstream: https://github.com/python/cpython/issues/68966

Tracker bug: https://bugzilla.redhat.com/show_bug.cgi?id=2075390

Backported from python3.
---
 Doc/library/mailcap.rst                       |  12 +
 Lib/mailcap.py                                |  29 +-
 Lib/test/mailcap.txt                          |  39 +++
 Lib/test/test_mailcap.py                      | 259 ++++++++++++++++++
 ...2-04-27-18-25-30.gh-issue-68966.gjS8zs.rst |   4 +
 5 files changed, 341 insertions(+), 2 deletions(-)
 create mode 100644 Lib/test/mailcap.txt
 create mode 100644 Lib/test/test_mailcap.py
 create mode 100644 Misc/NEWS.d/next/Security/2022-04-27-18-25-30.gh-issue-68966.gjS8zs.rst

diff --git a/Doc/library/mailcap.rst b/Doc/library/mailcap.rst
index 750d085796f..5f75ee6086e 100644
--- a/Doc/library/mailcap.rst
+++ b/Doc/library/mailcap.rst
@@ -54,6 +54,18 @@ standard.  However, mailcap files are supported on most Unix systems.
    use) to determine whether or not the mailcap line applies.  :func:`findmatch`
    will automatically check such conditions and skip the entry if the check fails.
 
+   .. versionchanged:: 3.11
+
+      To prevent security issues with shell metacharacters (symbols that have
+      special effects in a shell command line), ``findmatch`` will refuse
+      to inject ASCII characters other than alphanumerics and ``@+=:,./-_``
+      into the returned command line.
+
+      If a disallowed character appears in *filename*, ``findmatch`` will always
+      return ``(None, None)`` as if no entry was found.
+      If such a character appears elsewhere (a value in *plist* or in *MIMEtype*),
+      ``findmatch`` will ignore all mailcap entries which use that value.
+      A :mod:`warning <warnings>` will be raised in either case.
 
 .. function:: getcaps()
 
diff --git a/Lib/mailcap.py b/Lib/mailcap.py
index 04077ba0db2..1108b447b1d 100644
--- a/Lib/mailcap.py
+++ b/Lib/mailcap.py
@@ -1,9 +1,18 @@
 """Mailcap file handling.  See RFC 1524."""
 
 import os
+import warnings
+import re
 
 __all__ = ["getcaps","findmatch"]
 
+
+_find_unsafe = re.compile(r'[^\xa1-\xff\w@+=:,./-]').search
+
+class UnsafeMailcapInput(Warning):
+    """Warning raised when refusing unsafe input"""
+
+
 # Part 1: top-level interface.
 
 def getcaps():
@@ -144,15 +153,22 @@ def findmatch(caps, MIMEtype, key='view', filename="/dev/null", plist=[]):
     entry to use.
 
     """
+    if _find_unsafe(filename):
+        msg = "Refusing to use mailcap with filename %r. Use a safe temporary filename." % (filename,)
+        warnings.warn(msg, UnsafeMailcapInput)
+        return None, None
     entries = lookup(caps, MIMEtype, key)
     # XXX This code should somehow check for the needsterminal flag.
     for e in entries:
         if 'test' in e:
             test = subst(e['test'], filename, plist)
+            if test is None:
+                continue
             if test and os.system(test) != 0:
                 continue
         command = subst(e[key], MIMEtype, filename, plist)
-        return command, e
+        if command is not None:
+            return command, e
     return None, None
 
 def lookup(caps, MIMEtype, key=None):
@@ -184,6 +200,10 @@ def subst(field, MIMEtype, filename, plist=[]):
             elif c == 's':
                 res = res + filename
             elif c == 't':
+                if _find_unsafe(MIMEtype):
+                    msg = "Refusing to substitute MIME type %r into a shell command." % (MIMEtype,)
+                    warnings.warn(msg, UnsafeMailcapInput)
+                    return None
                 res = res + MIMEtype
             elif c == '{':
                 start = i
@@ -191,7 +211,12 @@ def subst(field, MIMEtype, filename, plist=[]):
                     i = i+1
                 name = field[start:i]
                 i = i+1
-                res = res + findparam(name, plist)
+                param = findparam(name, plist)
+                if _find_unsafe(param):
+                    msg = "Refusing to substitute parameter %r (%s) into a shell command" % (param, name)
+                    warnings.warn(msg, UnsafeMailcapInput)
+                    return None
+                res = res + param
             # XXX To do:
             # %n == number of parts if type is multipart/*
             # %F == list of alternating type and filename for parts
diff --git a/Lib/test/mailcap.txt b/Lib/test/mailcap.txt
new file mode 100644
index 00000000000..08a76e65941
--- /dev/null
+++ b/Lib/test/mailcap.txt
@@ -0,0 +1,39 @@
+# Mailcap file for test_mailcap; based on RFC 1524
+# Referred to by test_mailcap.py
+
+#
+# This is a comment.
+#
+
+application/frame; showframe %s; print="cat %s | lp"
+application/postscript; ps-to-terminal %s;\
+    needsterminal
+application/postscript; ps-to-terminal %s; \
+    compose=idraw %s
+application/x-dvi; xdvi %s
+application/x-movie; movieplayer %s; compose=moviemaker %s; \
+       description="Movie"; \
+       x11-bitmap="/usr/lib/Zmail/bitmaps/movie.xbm"
+application/*; echo "This is \"%t\" but \
+       is 50 \% Greek to me" \; cat %s; copiousoutput
+
+audio/basic; showaudio %s; compose=audiocompose %s; edit=audiocompose %s;\
+description="An audio fragment"
+audio/* ; /usr/local/bin/showaudio %t
+
+image/rgb; display %s
+#image/gif; display %s
+image/x-xwindowdump; display %s
+
+# The continuation char shouldn't \
+# make a difference in a comment.
+
+message/external-body; showexternal %s %{access-type} %{name} %{site} \
+    %{directory} %{mode} %{server}; needsterminal; composetyped = extcompose %s; \
+    description="A reference to data stored in an external location"
+
+text/richtext; shownonascii iso-8859-8 -e richtext -p %s; test=test "`echo \
+    %{charset} | tr '[A-Z]' '[a-z]'`"  = iso-8859-8; copiousoutput
+
+video/*; animate %s
+video/mpeg; mpeg_play %s
\ No newline at end of file
diff --git a/Lib/test/test_mailcap.py b/Lib/test/test_mailcap.py
new file mode 100644
index 00000000000..35da7fb0741
--- /dev/null
+++ b/Lib/test/test_mailcap.py
@@ -0,0 +1,259 @@
+import copy
+import os
+import sys
+import test.support
+import unittest
+from test import support as os_helper
+from test import support as warnings_helper
+from collections import OrderedDict
+
+import mailcap
+
+
+# Location of mailcap file
+MAILCAPFILE = test.support.findfile("mailcap.txt")
+
+# Dict to act as mock mailcap entry for this test
+# The keys and values should match the contents of MAILCAPFILE
+
+MAILCAPDICT = {
+    'application/x-movie':
+        [{'compose': 'moviemaker %s',
+          'x11-bitmap': '"/usr/lib/Zmail/bitmaps/movie.xbm"',
+          'description': '"Movie"',
+          'view': 'movieplayer %s',
+          'lineno': 4}],
+    'application/*':
+        [{'copiousoutput': '',
+          'view': 'echo "This is \\"%t\\" but        is 50 \\% Greek to me" \\; cat %s',
+          'lineno': 5}],
+    'audio/basic':
+        [{'edit': 'audiocompose %s',
+          'compose': 'audiocompose %s',
+          'description': '"An audio fragment"',
+          'view': 'showaudio %s',
+          'lineno': 6}],
+    'video/mpeg':
+        [{'view': 'mpeg_play %s', 'lineno': 13}],
+    'application/postscript':
+        [{'needsterminal': '', 'view': 'ps-to-terminal %s', 'lineno': 1},
+         {'compose': 'idraw %s', 'view': 'ps-to-terminal %s', 'lineno': 2}],
+    'application/x-dvi':
+        [{'view': 'xdvi %s', 'lineno': 3}],
+    'message/external-body':
+        [{'composetyped': 'extcompose %s',
+          'description': '"A reference to data stored in an external location"',
+          'needsterminal': '',
+          'view': 'showexternal %s %{access-type} %{name} %{site}     %{directory} %{mode} %{server}',
+          'lineno': 10}],
+    'text/richtext':
+        [{'test': 'test "`echo     %{charset} | tr \'[A-Z]\' \'[a-z]\'`"  = iso-8859-8',
+          'copiousoutput': '',
+          'view': 'shownonascii iso-8859-8 -e richtext -p %s',
+          'lineno': 11}],
+    'image/x-xwindowdump':
+        [{'view': 'display %s', 'lineno': 9}],
+    'audio/*':
+        [{'view': '/usr/local/bin/showaudio %t', 'lineno': 7}],
+    'video/*':
+        [{'view': 'animate %s', 'lineno': 12}],
+    'application/frame':
+        [{'print': '"cat %s | lp"', 'view': 'showframe %s', 'lineno': 0}],
+    'image/rgb':
+        [{'view': 'display %s', 'lineno': 8}]
+}
+
+# In Python 2, mailcap doesn't return line numbers.
+# This test suite is copied from Python 3.11; for easier backporting we keep
+# data from there and remove the lineno.
+# So, for Python 2, MAILCAPDICT_DEPRECATED is the same as MAILCAPDICT
+MAILCAPDICT_DEPRECATED = MAILCAPDICT
+for entry_list in MAILCAPDICT_DEPRECATED.values():
+    for entry in entry_list:
+        entry.pop('lineno')
+
+
+class HelperFunctionTest(unittest.TestCase):
+
+    def test_listmailcapfiles(self):
+        # The return value for listmailcapfiles() will vary by system.
+        # So verify that listmailcapfiles() returns a list of strings that is of
+        # non-zero length.
+        mcfiles = mailcap.listmailcapfiles()
+        self.assertIsInstance(mcfiles, list)
+        for m in mcfiles:
+            self.assertIsInstance(m, str)
+        with os_helper.EnvironmentVarGuard() as env:
+            # According to RFC 1524, if MAILCAPS env variable exists, use that
+            # and only that.
+            if "MAILCAPS" in env:
+                env_mailcaps = env["MAILCAPS"].split(os.pathsep)
+            else:
+                env_mailcaps = ["/testdir1/.mailcap", "/testdir2/mailcap"]
+                env["MAILCAPS"] = os.pathsep.join(env_mailcaps)
+                mcfiles = mailcap.listmailcapfiles()
+        self.assertEqual(env_mailcaps, mcfiles)
+
+    def test_readmailcapfile(self):
+        # Test readmailcapfile() using test file. It should match MAILCAPDICT.
+        with open(MAILCAPFILE, 'r') as mcf:
+                d = mailcap.readmailcapfile(mcf)
+        self.assertDictEqual(d, MAILCAPDICT_DEPRECATED)
+
+    def test_lookup(self):
+        # Test without key
+
+        # In Python 2, 'video/mpeg' is tried before 'video/*'
+        # (unfixed bug: https://github.com/python/cpython/issues/59182 )
+        # So, these are in reverse order:
+        expected = [{'view': 'mpeg_play %s', },
+                    {'view': 'animate %s', }]
+        actual = mailcap.lookup(MAILCAPDICT, 'video/mpeg')
+        self.assertListEqual(expected, actual)
+
+        # Test with key
+        key = 'compose'
+        expected = [{'edit': 'audiocompose %s',
+                     'compose': 'audiocompose %s',
+                     'description': '"An audio fragment"',
+                     'view': 'showaudio %s',
+                     }]
+        actual = mailcap.lookup(MAILCAPDICT, 'audio/basic', key)
+        self.assertListEqual(expected, actual)
+
+        # Test on user-defined dicts without line numbers
+        expected = [{'view': 'mpeg_play %s'}, {'view': 'animate %s'}]
+        actual = mailcap.lookup(MAILCAPDICT_DEPRECATED, 'video/mpeg')
+        self.assertListEqual(expected, actual)
+
+    def test_subst(self):
+        plist = ['id=1', 'number=2', 'total=3']
+        # test case: ([field, MIMEtype, filename, plist=[]], <expected string>)
+        test_cases = [
+            (["", "audio/*", "foo.txt"], ""),
+            (["echo foo", "audio/*", "foo.txt"], "echo foo"),
+            (["echo %s", "audio/*", "foo.txt"], "echo foo.txt"),
+            (["echo %t", "audio/*", "foo.txt"], None),
+            (["echo %t", "audio/wav", "foo.txt"], "echo audio/wav"),
+            (["echo \\%t", "audio/*", "foo.txt"], "echo %t"),
+            (["echo foo", "audio/*", "foo.txt", plist], "echo foo"),
+            (["echo %{total}", "audio/*", "foo.txt", plist], "echo 3")
+        ]
+        for tc in test_cases:
+            self.assertEqual(mailcap.subst(*tc[0]), tc[1])
+
+class GetcapsTest(unittest.TestCase):
+
+    def test_mock_getcaps(self):
+        # Test mailcap.getcaps() using mock mailcap file in this dir.
+        # Temporarily override any existing system mailcap file by pointing the
+        # MAILCAPS environment variable to our mock file.
+        with os_helper.EnvironmentVarGuard() as env:
+            env["MAILCAPS"] = MAILCAPFILE
+            caps = mailcap.getcaps()
+            self.assertDictEqual(caps, MAILCAPDICT)
+
+    def test_system_mailcap(self):
+        # Test mailcap.getcaps() with mailcap file(s) on system, if any.
+        caps = mailcap.getcaps()
+        self.assertIsInstance(caps, dict)
+        mailcapfiles = mailcap.listmailcapfiles()
+        existingmcfiles = [mcf for mcf in mailcapfiles if os.path.exists(mcf)]
+        if existingmcfiles:
+            # At least 1 mailcap file exists, so test that.
+            for (k, v) in caps.items():
+                self.assertIsInstance(k, str)
+                self.assertIsInstance(v, list)
+                for e in v:
+                    self.assertIsInstance(e, dict)
+        else:
+            # No mailcap files on system. getcaps() should return empty dict.
+            self.assertEqual({}, caps)
+
+
+class FindmatchTest(unittest.TestCase):
+
+    def test_findmatch(self):
+
+        # default findmatch arguments
+        c = MAILCAPDICT
+        fname = "foo.txt"
+        plist = ["access-type=default", "name=john", "site=python.org",
+                 "directory=/tmp", "mode=foo", "server=bar"]
+        audio_basic_entry = {
+            'edit': 'audiocompose %s',
+            'compose': 'audiocompose %s',
+            'description': '"An audio fragment"',
+            'view': 'showaudio %s',
+        }
+        audio_entry = {"view": "/usr/local/bin/showaudio %t", }
+        video_entry = {'view': 'animate %s', }
+        mpeg_entry = {'view': 'mpeg_play %s', }
+        message_entry = {
+            'composetyped': 'extcompose %s',
+            'description': '"A reference to data stored in an external location"', 'needsterminal': '',
+            'view': 'showexternal %s %{access-type} %{name} %{site}     %{directory} %{mode} %{server}',
+        }
+
+        # test case: (findmatch args, findmatch keyword args, expected output)
+        #   positional args: caps, MIMEtype
+        #   keyword args: key="view", filename="/dev/null", plist=[]
+        #   output: (command line, mailcap entry)
+        cases = [
+            ([{}, "video/mpeg"], {}, (None, None)),
+            ([c, "foo/bar"], {}, (None, None)),
+
+            # In Python 2, 'video/mpeg' is tried before 'video/*'
+            # (unfixed bug: https://github.com/python/cpython/issues/59182 )
+            #([c, "video/mpeg"], {}, ('animate /dev/null', video_entry)),
+            ([c, "video/mpeg"], {}, ('mpeg_play /dev/null', mpeg_entry)),
+
+            ([c, "audio/basic", "edit"], {}, ("audiocompose /dev/null", audio_basic_entry)),
+            ([c, "audio/basic", "compose"], {}, ("audiocompose /dev/null", audio_basic_entry)),
+            ([c, "audio/basic", "description"], {}, ('"An audio fragment"', audio_basic_entry)),
+            ([c, "audio/basic", "foobar"], {}, (None, None)),
+            ([c, "video/*"], {"filename": fname}, ("animate %s" % fname, video_entry)),
+            ([c, "audio/basic", "compose"],
+             {"filename": fname},
+             ("audiocompose %s" % fname, audio_basic_entry)),
+            ([c, "audio/basic"],
+             {"key": "description", "filename": fname},
+             ('"An audio fragment"', audio_basic_entry)),
+            ([c, "audio/*"],
+             {"filename": fname},
+             (None, None)),
+            ([c, "audio/wav"],
+             {"filename": fname},
+             ("/usr/local/bin/showaudio audio/wav", audio_entry)),
+            ([c, "message/external-body"],
+             {"plist": plist},
+             ("showexternal /dev/null default john python.org     /tmp foo bar", message_entry))
+        ]
+        self._run_cases(cases)
+
+    @unittest.skipUnless(os.name == "posix", "Requires 'test' command on system")
+    @unittest.skipIf(sys.platform == "vxworks", "'test' command is not supported on VxWorks")
+    def test_test(self):
+        # findmatch() will automatically check any "test" conditions and skip
+        # the entry if the check fails.
+        caps = {"test/pass": [{"test": "test 1 -eq 1"}],
+                "test/fail": [{"test": "test 1 -eq 0"}]}
+        # test case: (findmatch args, findmatch keyword args, expected output)
+        #   positional args: caps, MIMEtype, key ("test")
+        #   keyword args: N/A
+        #   output: (command line, mailcap entry)
+        cases = [
+            # findmatch will return the mailcap entry for test/pass because it evaluates to true
+            ([caps, "test/pass", "test"], {}, ("test 1 -eq 1", {"test": "test 1 -eq 1"})),
+            # findmatch will return None because test/fail evaluates to false
+            ([caps, "test/fail", "test"], {}, (None, None))
+        ]
+        self._run_cases(cases)
+
+    def _run_cases(self, cases):
+        for c in cases:
+            self.assertEqual(mailcap.findmatch(*c[0], **c[1]), c[2])
+
+
+def test_main():
+    test.support.run_unittest(HelperFunctionTest, GetcapsTest, FindmatchTest)
diff --git a/Misc/NEWS.d/next/Security/2022-04-27-18-25-30.gh-issue-68966.gjS8zs.rst b/Misc/NEWS.d/next/Security/2022-04-27-18-25-30.gh-issue-68966.gjS8zs.rst
new file mode 100644
index 00000000000..da81a1f6993
--- /dev/null
+++ b/Misc/NEWS.d/next/Security/2022-04-27-18-25-30.gh-issue-68966.gjS8zs.rst
@@ -0,0 +1,4 @@
+The deprecated mailcap module now refuses to inject unsafe text (filenames,
+MIME types, parameters) into shell commands. Instead of using such text, it
+will warn and act as if a match was not found (or for test commands, as if
+the test failed).