-
-
Notifications
You must be signed in to change notification settings - Fork 30.5k
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
[CVE-2015-20107] mailcap.findmatch: document shell command Injection danger in filename parameter #68966
Comments
if the filename contains Shell Commands they will be executed if they https://docs.python.org/2/library/mailcap.html "mailcap.findmatch(/caps/, /MIMEtype/[, /key/[, /filename/[, /plist/]]])
......" Exploid Demo wich runs xterm but should not : import mailcap
d=mailcap.getcaps()
commandline,MIMEtype=mailcap.findmatch(d, "text/*", filename="'$(xterm);#.txt")
## commandline = "less ''$(xterm);#.txt'"
import os
os.system(commandline)
## xterm starts ============================= By the way ... please do not use os.system() in your code, makes it unsafe. Best regards |
Maybe it would be a good idea to do so as run-mailcap does : theregrunner@mint17 : ~ € run-mailcap --debug "';xterm;#'.txt"
|
In this case os.system is an appropriate API, because it mirrors the API of mailcap itself (that is, mailcap entries are shell commands). I'm not convinced there is a security bug here. It seems to me that there are two cases: either the filename is determined by the program, in which case there is no security issue, or the filename comes from an external source, and the program will have had to *write it to the file system* before the mailcap command will do anything. So the security hole, if any, will have happened earlier in the process. Now, one can argue that the quoting should be done in order to preserve the meaning of an arbitrary filename. Which would allay your concern even if I disagree that it is a real security bug :) (I don't understand why run-mailcap uses an alias rather than correctly quoting the meta-characters.) |
@david I think if you read the Documentation You ask why run-mailcap do not use quotig, i believe because quoting is not an easy thing to do, i attached a demo ;-) Thank you. |
Exploid Demo wich works with quote() : >>> commandline,MIMETYPE=mailcap.findmatch(d, 'text/*', filename=quote(';xterm;#.txt'))
>>> commandline
"less '';xterm;#.txt''"
>>> os.system(commandline)
### xterm starts |
Hmm. I see. The problem is that our desire to quote conflicts with mailcap's attempts to quote. I now agree with you that run-mailcap's approach is correct, but creating a temporary alias is out of scope for findmatch. That would need to be done by findmatch's caller. I think we should add a documentation note about the problem and the solution. I don't see any reliable way to detect the problem and raise an error for the same reason that quoting doesn't work. (The aliasing can tolerate false positives; but, for backward compatibility reasons, an error detection function here cannot.) It would be possible to add a helper for the aliasing to 3.6, but if someone wants to propose that they should open an new issue for the enhancement. I'm |
Yes changing the docs is a good idea. I was thinking about a patch : import os
####### patch
import random
try:
from shlex import quote
except ImportError:
from pipes import quote
####### ....... and so on .... # Part 3: using the database. def findmatch(caps, MIMEtype, key='view', filename="/dev/null", plist=[]):
"""Find a match for a mailcap entry.
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 and os.system(test) != 0:
continue
####### patch
ps=''.join(random.choice('python') for i in range(100))
x=e[key]
while '%s' in x:
x=x.replace('%s',ps)
command=subst(x, MIMEtype, filename, plist)
while "'"+ps+"'" in command:
command=command.replace("'"+ps+"'",quote(filename))
while ps in command:
command=command.replace(ps,quote(filename))
###### command = subst(e[key], MIMEtype, filename, plist)
return command, e
return None, None |
# for the docs ... quoting of the filename when you call mailcap.findmatch() f=";xterm;#.txt" # Shell Command Demo ... xterm will run if quote() fails import mailcap
import random
try:
from shlex import quote
except ImportError:
from pipes import quote
d=mailcap.getcaps()
PY=''.join(random.choice('PYTHON') for i in range(100))
cmd,MIMEtype=mailcap.findmatch(d, 'text/plain', filename=PY)
while "'"+PY+"'" in cmd:
cmd=cmd.replace("'"+PY+"'",quote(f))
while PY in cmd:
cmd=cmd.replace(PY,quote(f))
print(cmd)
# less ';xterm;#.txt' |
I have no idea what your code samples are trying to accomplish, I'm afraid, but that's not the kind of documentation I'm advocating anyway. |
What i do is the last doc is like this :
|
Ah, that's a clever idea. |
Thanks :-) As you may noticed i now choosed to use a random name made of the chars of "PYTHON" in BIG letters instead of small letters i used before. Thats because i do not want to get in trouble with the little "t" in %t wich is replaced by the subst function too. |
My patch for mailcap.py. Please check and apply my patch please.
Thank you. |
In 2022, Python 3.11 still has the issue: vstinner@apu$ python3.11 -m mailcap
Mailcap files:
/home/vstinner/.mailcap
/etc/mailcap
(...)
Mailcap entries:
(...)
text/html
copiousoutput
lineno 5
view /usr/bin/xdg-open %s
$ python3 -m mailcap text/html 'filename; pwd'
Executing: /usr/bin/xdg-open filename; pwd
(...)
/home/vstinner/python/main Maybe subst() can be modified to work on a list (as Bernd Dietzel proposed) and then use subprocess to avoid shell and so avoid having to pass a single string, but pass a *list* The problem is that it would change the public mailcap.findmatch() API: Adding a new findmatch_list() function avoids the backward compatibility issue, but the existing findmatch() function would remain vulnerable. The other problem is that the mailcap.findmatch() function supports "test" command which Is the mailcap format (RFC 1524) still used in 2022? Does the mailcap module still belong to the Python stdlib in 2022? I propose to:
A code search in the top 5000 PyPI projects (at 2022-01-26) did not find any Python source code using the "mailcap" module. I only found the word "mailcap" used to refer to other things:
https://docs.djangoproject.com/en/4.0/ref/contrib/staticfiles/ |
It's too bad this didn't make it into PEP 594, but I think we can deprecate it anyway unless someone steps up to be a maintainer. The original patch doesn't add quotes in a way that solves the problem without potentially breaking legitimate users. The best mitigation is to validate user-provided values before using them, in this case, |
FYI, this issue now has the CVE ID Notification at https://mail.python.org/archives/list/[email protected]/thread/QDSXNCW77UGULFG2JMDFZQ7H4DIR32LA/ |
I vote to deprecate. I guess we just need to ask on python-dev/committers to see if anyone will maintain it and what the community impact may be? |
Hi all, i am the one raised the issue that the security issue was not fixed 7 years ago - you are welcomed to address me regarding the mitigation. Vulnerability Description: Command Structure Steps to reproduce (in Linux)
The Payload The vulnerability in general Exploitation
Leafpad application will open immediately. -- Dan Shallom Thanks, Dan |
I've proposed deprecating mailcap at https://mail.python.org/archives/list/[email protected]/thread/EB2BS4DBWSTBIOPQL5QTBSIOBORWSCMJ/ . |
@brettcannon Indeed, 3.10-3.12 need to get a fix.
|
The patch is released in 3.10.8. 3.7 - 3.9 releases that contain the patch in question are coming on December 5th. The backports missed the previous round of releases. |
First thank you for your help here and the work on the project. We also found merge #98192 from Oct 11 plus above mentioned from Oct 19. Is there somewhere an explanation of the release flow from merges into AnacondaRecipes/python-feedstock and python/cpython -- all the way to released python versions? It would make these investigations self-serve. :) Thank you |
No idea what Anaconda does, you need to ask them. As for CPython, our process is that a fix lands in Per PEP 619 the next 3.10 bugfix release is scheduled for December 5th and the other release managers synchronized their calendars to release 3.7 - 3.12 on that day. |
https://python-security.readthedocs.io/vuln/mailcap-shell-injection.html seems to be up to date, no? Fixed In
Vulnerable Versions
@ambv announced the next batch of releases. In general, I look at release PEPs in https://devguide.python.org/versions/ for estimated release dates. |
Hey, can you update the Known Affected Software Configurations (CPE) in the CVE to the correct one as mentioned below and also mention the older unfixed versions in the CPE as well ?
Also @vstinner , It seems like https://python-security.readthedocs.io/vuln/mailcap-shell-injection.html is not up-to-date - "In Python (aka CPython) through 3.10.4". |
My tool just copies what the CVE says. |
* Post 3.8.16 * [3.8] Update copyright years to 2023. (pythongh-100852) * [3.8] Update copyright years to 2023. (pythongh-100848). (cherry picked from commit 11f9932) Co-authored-by: Benjamin Peterson <[email protected]> * Update additional copyright years to 2023. Co-authored-by: Ned Deily <[email protected]> * [3.8] Update copyright year in README (pythonGH-100863) (pythonGH-100867) (cherry picked from commit 30a6cc4) Co-authored-by: Ned Deily <[email protected]> Co-authored-by: HARSHA VARDHAN <[email protected]> * [3.8] Correct CVE-2020-10735 documentation (pythonGH-100306) (python#100698) (cherry picked from commit 1cf3d78) (cherry picked from commit 88fe8d7) Co-authored-by: Jeremy Paige <[email protected]> Co-authored-by: Gregory P. Smith <[email protected]> * [3.8] Bump Azure Pipelines to ubuntu-22.04 (pythonGH-101089) (python#101215) (cherry picked from commit c22a55c) Co-authored-by: Hugo van Kemenade <[email protected]> * [3.8] pythongh-100180: Update Windows installer to OpenSSL 1.1.1s (pythonGH-100903) (python#101258) * pythongh-101422: (docs) TarFile default errorlevel argument is 1, not 0 (pythonGH-101424) (cherry picked from commit ea23271) Co-authored-by: Owain Davies <[email protected]> * [3.8] pythongh-95778: add doc missing in some places (pythonGH-100627) (python#101630) (cherry picked from commit 4652182) * [3.8] pythongh-101283: Improved fallback logic for subprocess with shell=True on Windows (pythonGH-101286) (python#101710) Co-authored-by: Oleg Iarygin <[email protected]> Co-authored-by: Steve Dower <[email protected]> * [3.8] pythongh-101981: Fix Ubuntu SSL tests with OpenSSL (3.1.0-beta1) CI i… (python#102095) [3.8] pythongh-101981: Fix Ubuntu SSL tests with OpenSSL (3.1.0-beta1) CI issue (pythongh-102079) * [3.8] pythonGH-102306 Avoid GHA CI macOS test_posix failure by using the appropriate macOS SDK (pythonGH-102307) [3.8] Avoid GHA CI macOS test_posix failure by using the appropriate macOS SDK. * [3.8] pythongh-101726: Update the OpenSSL version to 1.1.1t (pythonGH-101727) (pythonGH-101752) Fixes CVE-2023-0286 (High) and a couple of Medium security issues. https://www.openssl.org/news/secadv/20230207.txt Co-authored-by: Gregory P. Smith <[email protected]> Co-authored-by: Ned Deily <[email protected]> * [3.8] pythongh-102627: Replace address pointing toward malicious web page (pythonGH-102630) (pythonGH-102667) (cherry picked from commit 61479d4) Co-authored-by: Blind4Basics <[email protected]> Co-authored-by: C.A.M. Gerlach <[email protected]> Co-authored-by: Hugo van Kemenade <[email protected]> * [3.8] pythongh-101997: Update bundled pip version to 23.0.1 (pythonGH-101998). (python#102244) (cherry picked from commit 89d9ff0) * [3.8] pythongh-102950: Implement PEP 706 – Filter for tarfile.extractall (pythonGH-102953) (python#104548) Backport of c8c3956 * [3.8] pythongh-99889: Fix directory traversal security flaw in uu.decode() (pythonGH-104096) (python#104332) (cherry picked from commit 0aeda29) Co-authored-by: Sam Carroll <[email protected]> * [3.8] pythongh-104049: do not expose on-disk location from SimpleHTTPRequestHandler (pythonGH-104067) (python#104121) Do not expose the local server's on-disk location from `SimpleHTTPRequestHandler` when generating a directory index. (unnecessary information disclosure) (cherry picked from commit c7c3a60) Co-authored-by: Ethan Furman <[email protected]> Co-authored-by: Gregory P. Smith <[email protected]> Co-authored-by: Jelle Zijlstra <[email protected]> * [3.8] pythongh-103935: Use `io.open_code()` when executing code in trace and profile modules (pythonGH-103947) (python#103954) Co-authored-by: Tian Gao <[email protected]> * [3.8] pythongh-68966: fix versionchanged in docs (pythonGH-105299) * [3.8] Update GitHub CI workflow for macOS. (pythonGH-105302) * [3.8] pythongh-105184: document that marshal functions can fail and need to be checked with PyErr_Occurred (pythonGH-105185) (python#105222) (cherry picked from commit ee26ca1) Co-authored-by: Irit Katriel <[email protected]> * [3.8] pythongh-102153: Start stripping C0 control and space chars in `urlsplit` (pythonGH-102508) (pythonGH-104575) (pythonGH-104592) (python#104593) (python#104895) `urllib.parse.urlsplit` has already been respecting the WHATWG spec a bit pythonGH-25595. This adds more sanitizing to respect the "Remove any leading C0 control or space from input" [rule](https://url.spec.whatwg.org/GH-url-parsing:~:text=Remove%20any%20leading%20and%20trailing%20C0%20control%20or%20space%20from%20input.) in response to [CVE-2023-24329](https://nvd.nist.gov/vuln/detail/CVE-2023-24329). I simplified the docs by eliding the state of the world explanatory paragraph in this security release only backport. (people will see that in the mainline /3/ docs) (cherry picked from commit d7f8a5f) (cherry picked from commit 2f630e1) (cherry picked from commit 610cc0a) (cherry picked from commit f48a96a) Co-authored-by: Miss Islington (bot) <[email protected]> Co-authored-by: Illia Volochii <[email protected]> Co-authored-by: Gregory P. Smith [Google] <[email protected]> * [3.8] pythongh-103142: Upgrade binary builds and CI to OpenSSL 1.1.1u (pythonGH-105174) (pythonGH-105200) (pythonGH-105205) (python#105370) Upgrade builds to OpenSSL 1.1.1u. Also updates _ssl_data_111.h from OpenSSL 1.1.1u, _ssl_data_300.h from 3.0.9. Manual edits to the _ssl_data_300.h file prevent it from removing any existing definitions in case those exist in some peoples builds and were important (avoiding regressions during backporting). (cherry picked from commit ede89af) (cherry picked from commit e15de14) Co-authored-by: Gregory P. Smith <[email protected]> Co-authored-by: Ned Deily <[email protected]> * Python 3.8.17 * Post 3.8.17 * Updated CI to build 3.8.17 --------- Co-authored-by: Łukasz Langa <[email protected]> Co-authored-by: Benjamin Peterson <[email protected]> Co-authored-by: Ned Deily <[email protected]> Co-authored-by: Miss Islington (bot) <[email protected]> Co-authored-by: HARSHA VARDHAN <[email protected]> Co-authored-by: Gregory P. Smith <[email protected]> Co-authored-by: Jeremy Paige <[email protected]> Co-authored-by: Hugo van Kemenade <[email protected]> Co-authored-by: Steve Dower <[email protected]> Co-authored-by: Owain Davies <[email protected]> Co-authored-by: Éric <[email protected]> Co-authored-by: Oleg Iarygin <[email protected]> Co-authored-by: Steve Dower <[email protected]> Co-authored-by: Dong-hee Na <[email protected]> Co-authored-by: Blind4Basics <[email protected]> Co-authored-by: C.A.M. Gerlach <[email protected]> Co-authored-by: Pradyun Gedam <[email protected]> Co-authored-by: Petr Viktorin <[email protected]> Co-authored-by: Sam Carroll <[email protected]> Co-authored-by: Ethan Furman <[email protected]> Co-authored-by: Jelle Zijlstra <[email protected]> Co-authored-by: Tian Gao <[email protected]> Co-authored-by: Irit Katriel <[email protected]> Co-authored-by: stratakis <[email protected]> Co-authored-by: Illia Volochii <[email protected]>
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
…params Avoid the command injection in mailcap. Patch is based on gh#python/cpython!91993, it was released upstream in 3.7.16. Fixes: bsc#1198511 Fixes: gh#python#68966 Patch: CVE-2015-20107-mailcap-unsafe-filenames.patch
…params Avoid the command injection in mailcap. Patch is based on gh#python/cpython!91993, it was released upstream in 3.7.16. Fixes: bsc#1198511 Fixes: gh#python#68966 Patch: CVE-2015-20107-mailcap-unsafe-filenames.patch
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
Linked PRs
The text was updated successfully, but these errors were encountered: