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/security-announce@python.org/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/python-dev@python.org/thread/EB2BS4DBWSTBIOPQL5QTBSIOBORWSCMJ/ . |
@brettcannon Indeed, 3.10-3.12 need to get a fix.
|
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
There are warnings from a few tests, I believe they are related to this issue.
|
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
Will this change be backported to 3.9.x as a security fix ? When is this change expected to be released ? |
Oh right, only 3.11 and main branches are fixed yet. The fix should be backported to other Python versions: https://devguide.python.org/versions/ |
…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>
…arams (pythonGH-91993) (cherry picked from commit b9509ba) Co-authored-by: Petr Viktorin <encukou@gmail.com>
Fixed in 3.7 - 3.12. |
I created https://python-security.readthedocs.io/vuln/mailcap-shell-injection.html to track this vulnerability. |
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
TheRegRunner mannequin commentedAug 2, 2015
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:
The text was updated successfully, but these errors were encountered: