On 22Jun2019 12:24, vincent lefevre <vinc...@vinc17.org> wrote:
FYI, due to incorrect mailcap rules, which use '%s' or similar
instead of just %s, the filename quoting system in Mutt eventually
makes the filename *unquoted*, i.e. reversing its purpose, e.g.

 "less ''/var/tmp/_.txt''"

I've reported a general bug in Debian:

 https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=930908

Just a tangent because we've been talking about sanitising filenames elsewhere...

Interestingly, a reading of mailcap(5) on Ubuntu and of RFC 1524 shows that it says _nothing_ about shell quoting.

All the text says that %s shall be replaced with a filename containing the body (or attachment). Now, clearly that needs to be Bourne shell safe, but the text of the RFC and the manual entry say nothing about how that is achieved.

My inference from reading them is that the supplied filename is itself shell safe without quotes - it would need to contain no shell punctuation or whitespace. And in that form, it wouldn't matter if the mailcap recipe contained quotes:

 less '%s'

works as:

 less 'foo.txt'

and a recipe without quotes:

 less %s

works as:

 less foo.txt

with such a filename.

Of course, _all_ the mailcap examples in both these documents are without quotes, suggesting that a tool can put in a shell quoted string safely as mutt endeavours to do, allow it to preserve a presupplied filename closely.

Returning to the quotes-in-mailcap-recipes issue, I'd be all for mutt noticing _and warning_ about mailcap entries with '%s' in them, and maybe doing an aggressive filename sanitisation at that point to provide an _unquoted_ but safe filename regardless of the source filename. One which would be safe in quotes or not.

Our obligation here is to pass the filename correctly to the recipe command. The preferred way is a shell-quoted string replacing %s because that lets us preserve any suggested/provided filename most accurately. But we _can_ notice a '%s' easily and provide a safe-if-quoted-or-unquoted filename, and I think therefore that we ought to.

[... mutt code specifics snipped ...]
But it appears that the filename is usually or always sanitized.
The code is not enough documented about that.

If the filename is expected to be always sanitized, this should
probably be double-checked here to be sure and potentially avoid
future security bugs.

I think we should try to be robust against '%s' in a mailcap recipe if we notice it, because we can provide an unquoted-and-more-sanitised filename in that circumstance which won't be misused but a recipe with erroneous extra quotes.

I'm happy to try to make some time to understand the mutt code and suggest a patch if there's agreement about this.

Cheers,
Cameron Simpson <c...@cskk.id.au>

Reply via email to