-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gh-68966: Make mailcap refuse to match unsafe filenames/types/params #91993
Conversation
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually when a warning is displayed, the function still returns its regular result. For example, calling a deprecated function emits a DeprecationWarning, but the function is executed. If we go this way (reject filenames which look unsafe), I suggest to raise an exception instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, IMO since the caller should always do whatever it does on None
result -- fall back to some other mechanism, or ask the user -- returning None is less disruptive.
Lib/mailcap.py
Outdated
@@ -19,6 +20,11 @@ def lineno_sort_key(entry): | |||
else: | |||
return 1, 0 | |||
|
|||
_find_unsafe = re.compile(r'[^\xa1-\U0010FFFF\w@%+=:,./-]').search |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is %
safe? DOS derived shells use that for variable expansion IIRC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mailcap implements the “Implementation Details for UNIX” appendix of RFC-1524 (which is what specifies using %
for the placeholders), and that says:
(Because of differences in shells and the implementation and behavior
of the same shell from one system to another, it is specified that
the command line be intended as input to the Bourne shell, i.e., that
it is implicitly preceded by "/bin/sh -c " on the command line.)
So I'd say it's safe for Python to allow %
. But it also won't hurt much to disallow it, so I'll do that.
…cond argument Source: python/cpython#91993 MR: 117402 Type: Security Fix Disposition: Backport from python/cpython@c3e7f13 ChangeID: 7118c5678a340cfcad95b0e5fb116b8919e389b8 Description: CVE-2015-20107 python(mailcap): findmatch() function does not sanitise the second argument. Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com> Signed-off-by: Jeremy A. Puhlman <jpuhlman@mvista.com>
I think it's good and it seems to work well. I like the regex. |
Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11. |
…arams (pythonGH-91993) (cherry picked from commit b9509ba) Co-authored-by: Petr Viktorin <encukou@gmail.com>
GH-93458 is a backport of this pull request to the 3.11 branch. |
…cond argument Source: python/cpython#91993 MR: 117410 Type: Security Fix Disposition: Backport from python/cpython@c3e7f13 ChangeID: 6101cf28d6a5288fe07c654df016c3f5810c705a Description: CVE-2015-20107 python(mailcap): findmatch() function does not sanitise the second argument. Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com> Signed-off-by: Jeremy A. Puhlman <jpuhlman@mvista.com>
Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10. |
GH-93543 is a backport of this pull request to the 3.10 branch. |
GH-98192 is a backport of this pull request to the 3.8 branch. |
…arams (pythonGH-91993) (cherry picked from commit b9509ba) Co-authored-by: Petr Viktorin <encukou@gmail.com>
…arams (pythonGH-91993) (cherry picked from commit b9509ba) Co-authored-by: Petr Viktorin <encukou@gmail.com>
Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7. |
…arams (pythonGH-91993) (cherry picked from commit b9509ba) Co-authored-by: Petr Viktorin <encukou@gmail.com>
#98191 is the 3.7 backport. Bedevere somehow missed the comment here. |
Make mailcap refuse to match unsafe filenames/types/params (pythonGH-91993) Upstream: python#68966 Tracker bug: https://bugzilla.redhat.com/show_bug.cgi?id=2075390
Make mailcap refuse to match unsafe filenames/types/params (pythonGH-91993) Upstream: python#68966 Tracker bug: https://bugzilla.redhat.com/show_bug.cgi?id=2075390
Make mailcap refuse to match unsafe filenames/types/params (pythonGH-91993) Upstream: python#68966 Tracker bug: https://bugzilla.redhat.com/show_bug.cgi?id=2075390
* Fix CVE-2015-20107 Implemented fix from python/cpython#91993 * Update regex Update regex to be Python 2 compatible.
00382 # Make mailcap refuse to match unsafe filenames/types/params (pythonGH-91993) Upstream: python#68966 Tracker bug: https://bugzilla.redhat.com/show_bug.cgi?id=2075390 Backported from python3.
Make mailcap refuse to match unsafe filenames/types/params (pythonGH-91993) Upstream: python#68966 Tracker bug: https://bugzilla.redhat.com/show_bug.cgi?id=2075390
Make mailcap refuse to match unsafe filenames/types/params (pythonGH-91993) Upstream: python#68966 Tracker bug: https://bugzilla.redhat.com/show_bug.cgi?id=2075390
Make mailcap refuse to match unsafe filenames/types/params (pythonGH-91993) Upstream: python#68966 Tracker bug: https://bugzilla.redhat.com/show_bug.cgi?id=2075390
Make mailcap refuse to match unsafe filenames/types/params (pythonGH-91993) Upstream: python#68966 Tracker bug: https://bugzilla.redhat.com/show_bug.cgi?id=2075390
Make mailcap refuse to match unsafe filenames/types/params (pythonGH-91993) Upstream: python#68966 Tracker bug: https://bugzilla.redhat.com/show_bug.cgi?id=2075390
Make mailcap refuse to match unsafe filenames/types/params (pythonGH-91993) Upstream: python#68966 Tracker bug: https://bugzilla.redhat.com/show_bug.cgi?id=2075390
Make mailcap refuse to match unsafe filenames/types/params (pythonGH-91993) Upstream: python#68966 Tracker bug: https://bugzilla.redhat.com/show_bug.cgi?id=2075390
Make mailcap refuse to match unsafe filenames/types/params (pythonGH-91993) Upstream: python#68966 Tracker bug: https://bugzilla.redhat.com/show_bug.cgi?id=2075390
Make mailcap refuse to match unsafe filenames/types/params (pythonGH-91993) Upstream: python#68966 Tracker bug: https://bugzilla.redhat.com/show_bug.cgi?id=2075390
Here is a possible fix for CVE-2015-20107 for cases where mailcap needs to continue working, at least when following best practices: refuse to inject text into commands unless deemed safe by a list of allowed characters.