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).