[issue24778] mailcap.findmatch: document shell command Injection danger in filename parameter

[STINNER Victor]

STINNER Victor  added the comment:

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*
of arguments (strings).

The problem is that it would change the public mailcap.findmatch() API:
"Return a 2-tuple; the first element is a string containing the command line to 
be executed (which can be passed to os.system()), (...)"
https://docs.python.org/dev/library/mailcap.html#mailcap.findmatch

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
executes os.system() on string created by mailcap.subst().

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:

* (1) Document the shell injection vulnerability: the caller is responsible to 
validate the filename
* (2) Deprecate the mailcap module


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/ mentions a 
"mailcap" RHEL package:

  "This can be achieved, for example, by installing or updating the mailcap 
package on a Red Hat distribution, mime-support on a Debian distribution, or by 
editing the keys under HKEY_CLASSES_ROOT in the Windows registry."

* wxPython refers to "KDE< mailcap and mime.types"

https://docs.djangoproject.com/en/4.0/ref/contrib/staticfiles/

--
nosy: +vstinner
versions: +Python 3.11 -Python 2.7, Python 3.5, Python 3.6, Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24778] mailcap.findmatch: document shell command Injection danger in filename parameter

[Christian Heimes]

Changes by Christian Heimes :


--
versions: +Python 3.7 -Python 3.4

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24778] mailcap.findmatch: document shell command Injection danger in filename parameter

[Bernd Dietzel]

Bernd Dietzel added the comment:

My patch for mailcap.py. Please check and apply my patch please.

1) I have removed the os.system() calls for security reasons.

2) New "findmtach_list()" function witch returns the commandline as a [list] 
witch can be passed to subprocess instead of passing it to os.system().

3) New run() function to execute the cmd_list with subprocess. 

4) The test() function now uses findmatch_list() and run() instead of the old 
findmatch() and os.system() calls.  

5) The subst() function is now shorter an does a quote(filename) when its 
replacing %s with a filename.

6) The "old" findmatch() function is still there if the user still likes to 
have the commandline as a "string". 
Attention ! With this old findmatch() function it's still possible that a shell 
command in the filename like '$(ls).txt' will be executed when the users passes 
the string to os.system() outside the mailcap script. Use findmatch() only for 
backwards compatibility.

7) Use the new findmatch_list() an run() for future projects.

8) Add 1)-7) to the docs

Thank you.

--
Added file: http://bugs.python.org/file40897/mailcap patch.zip

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24778] mailcap.findmatch: document shell command Injection danger in filename parameter

[R. David Murray]

R. David Murray added the comment:

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.

--
title: mailcap.findmatch()    Shell Command Injection in filename - 
mailcap.findmatch: document shell command Injection danger in filename parameter

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24778
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24778] mailcap.findmatch: document shell command Injection danger in filename parameter

[Bernd Dietzel]

Bernd Dietzel added the comment:

What i do is the last doc is like this :

1) Replace the filename with a random name
2) Run mailcap.findmatch() with the random name
3) If exists, replace the quote characters ' before and behind the random name 
with nothing.
4) Now the random name has no quoting from mailcap itself
5) So now we can use our own quote() savely

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24778
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24778] mailcap.findmatch: document shell command Injection danger in filename parameter

[R. David Murray]

R. David Murray added the comment:

Ah, that's a clever idea.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24778
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com