| From 0a4f650347fdcfd82d094ab2134ca89584f4e877 Mon Sep 17 00:00:00 2001 |
| From: "Miss Islington (bot)" |
| <31488909+miss-islington@users.noreply.github.com> |
| Date: Tue, 11 Oct 2022 14:58:03 -0700 |
| Subject: [PATCH] [3.8] gh-68966: Make mailcap refuse to match unsafe |
| filenames/types/params (GH-91993) (#98192) |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| gh-68966: Make mailcap refuse to match unsafe filenames/types/params (GH-91993) |
| (cherry picked from commit b9509ba7a9c668b984dab876c7926fe1dc5aa0ba) |
| |
| Co-authored-by: Petr Viktorin <encukou@gmail.com> |
| Co-authored-by: Łukasz Langa <lukasz@langa.pl> |
| --- |
| Doc/library/mailcap.rst | 12 +++++++++ |
| Lib/mailcap.py | 26 +++++++++++++++++-- |
| Lib/test/test_mailcap.py | 8 ++++-- |
| ...2-04-27-18-25-30.gh-issue-68966.gjS8zs.rst | 4 +++ |
| 4 files changed, 46 insertions(+), 4 deletions(-) |
| 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 bf9639bdaca5..a75857be623e 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 bd0fc0981c8c..dcd4b449e828 100644 |
| --- a/Lib/mailcap.py |
| +++ b/Lib/mailcap.py |
| @@ -2,6 +2,7 @@ |
| |
| import os |
| import warnings |
| +import re |
| |
| __all__ = ["getcaps","findmatch"] |
| |
| @@ -13,6 +14,11 @@ def lineno_sort_key(entry): |
| else: |
| return 1, 0 |
| |
| +_find_unsafe = re.compile(r'[^\xa1-\U0010FFFF\w@+=:,./-]').search |
| + |
| +class UnsafeMailcapInput(Warning): |
| + """Warning raised when refusing unsafe input""" |
| + |
| |
| # Part 1: top-level interface. |
| |
| @@ -165,15 +171,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): |
| @@ -206,6 +219,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 |
| @@ -213,7 +230,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/test_mailcap.py b/Lib/test/test_mailcap.py |
| index c08423c67073..920283d9a2e3 100644 |
| --- a/Lib/test/test_mailcap.py |
| +++ b/Lib/test/test_mailcap.py |
| @@ -121,7 +121,8 @@ def test_subst(self): |
| (["", "audio/*", "foo.txt"], ""), |
| (["echo foo", "audio/*", "foo.txt"], "echo foo"), |
| (["echo %s", "audio/*", "foo.txt"], "echo foo.txt"), |
| - (["echo %t", "audio/*", "foo.txt"], "echo audio/*"), |
| + (["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") |
| @@ -205,7 +206,10 @@ def test_findmatch(self): |
| ('"An audio fragment"', audio_basic_entry)), |
| ([c, "audio/*"], |
| {"filename": fname}, |
| - ("/usr/local/bin/showaudio audio/*", audio_entry)), |
| + (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)) |
| 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 000000000000..da81a1f6993d |
| --- /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). |