From: W. Trevor King <wking@tremily.us>
Date: Fri, 11 May 2012 13:48:27 +0000 (-0400)
Subject: Quote filenames before substituting into mailcap rules in mailcap-test.py.
X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=37f20b350167694e6605d6cab1e86fff8d0687c0;p=blog.git

Quote filenames before substituting into mailcap rules in mailcap-test.py.

Also quote the Mutt manpage in the associated blog post to explain why.
---

diff --git a/posts/mailcap.mdwn b/posts/mailcap.mdwn
index 99c7283..c10052a 100644
--- a/posts/mailcap.mdwn
+++ b/posts/mailcap.mdwn
@@ -8,10 +8,47 @@ entries: [[mailcap-test.py]].  Enjoy!
 Related utilities include [xdg-open][], [mimeopen][], and
 [sensible-utils][].
 
+Quoting
+-------
+
+Quoting in mailcap files is a tricky issue.  From the [Mutt][] man
+page:
+
+> Secure use of mailcap
+>
+> The interpretion of shell meta-characters embedded in MIME
+> parameters can lead to security problems in general. Mutt tries to
+> quote parameters in expansion of `%s` syntaxes properly, and avoids
+> risky characters by substituting them, see the `mailcap_sanitize`
+> variable.
+>
+> Although mutt's procedures to invoke programs with mailcap seem to
+> be safe, there are other applications parsing mailcap, maybe taking
+> less care of it. Therefore you should pay attention to the following
+> rules:
+>
+> *Keep the %-expandos away from shell quoting.* Don't quote them with
+> single or double quotes. Mutt does this for you, the right way, as
+> should any other program which interprets mailcap. Don't put them
+> into backtick expansions. Be highly careful with eval statements,
+> and avoid them if possible at all. Trying to fix broken behaviour
+> with quotes introduces new leaks - there is no alternative to
+> correct quoting in the first place.
+>
+> If you have to use the %-expandos' values in context where you need
+> quoting or backtick expansions, put that value into a shell variable
+> and reference the shell variable where necessary, as in the
+> following example (using $charset inside the backtick expansion is
+> safe, since it is not itself subject to any further expansion):
+>
+>     text/test-mailcap-bug; cat %s; copiousoutput; test=charset=%{charset} \
+>         && test "`echo $charset | tr '[A-Z]' '[a-z]'`" != iso-8859-1
+
 [rfc1524]: http://tools.ietf.org/html/rfc1524
 [xdg-open]: http://portland.freedesktop.org/xdg-utils-1.0/xdg-open.html
 [mimeopen]: http://search.cpan.org/dist/File-MimeInfo/mimeopen
 [sensible-utils]: http://packages.debian.org/sid/sensible-utils
+[Mutt]: http://www.mutt.org/doc/manual/manual-5.html
 
 [[!tag tags/code]]
 [[!tag tags/linux]]
diff --git a/posts/mailcap/mailcap-test.py b/posts/mailcap/mailcap-test.py
index 8990b7c..ee2b8ef 100755
--- a/posts/mailcap/mailcap-test.py
+++ b/posts/mailcap/mailcap-test.py
@@ -1,6 +1,6 @@
 #!/usr/bin/env python
 #
-# Copyright (C) 2011 W. Trevor King <wking@drexel.edu>
+# Copyright (C) 2011-2012 W. Trevor King <wking@drexel.edu>
 #
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU Lesser General Public License as
@@ -20,6 +20,10 @@
 
 import mailcap as _mailcap
 import mimetypes as _mimetypes
+import shlex as _shlex
+if not hasattr(_shlex, 'quote'):  # Python < 3.3
+    import pipes as _pipes
+    _shlex.quote = _pipes.quote
 import subprocess as _subprocess
 
 
@@ -31,7 +35,7 @@ def view(filename, mime=None):
         if mime is None:
             return 1
         print('guessed {} for {}'.format(mime, filename))
-    match = _mailcap.findmatch(_CAPS, mime, filename=filename)
+    match = _mailcap.findmatch(_CAPS, mime, filename=_shlex.quote(filename))
     if match[0] is None:
         return 1
     print('view {} with {}'.format(filename, match[0]))