From 0093fdeceff33e23efebcdf8971291c7f3bfa040 Mon Sep 17 00:00:00 2001 From: Petr Viktorin 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 750d085796f737..5f75ee6086ebd8 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 ` will be raised in either case. .. function:: getcaps() diff --git a/Lib/mailcap.py b/Lib/mailcap.py index 04077ba0db2cb4..1108b447b1db24 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 00000000000000..08a76e659410d9 --- /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 00000000000000..35da7fb0741586 --- /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=[]], ) + 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 00000000000000..da81a1f6993dbe --- /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).