On 2014-12-01 at 02:39 +0000, Viktor Dukhovni wrote: > On Mon, Dec 01, 2014 at 12:48:47AM +0100, Heiko Schlittermann wrote: > > the above line needs to be: > > > > av_scanner = cmdline:{ /bin/scan %s || true; }:<trigger>:<re> > > > > which is quite ugly. At least it should find it's way into the spec > > that we're using popen() and /bin/sh and that such commands will work. > > > > (I'm not sure about security implications from using popen() and > > /bin/sh). > > It largely boils down to what might be substituted for '%s'. Instead of > "true" I would use the ":" shell built-in: > > /bin/may_only_appear_to_fail || : ignore
The `%s` is directory-name created by Exim and is not influenced by anything in the email; it's "safe". scan_filename = string_sprintf("%s/scan/%s/%s.eml", spool_directory, message_id, message_id); The result of that gets truncated at the last `/` to get the directory-name to pass to the cmdline scanner. Since Exim configuration files must be owned by root, or a trusted user _other_ than the Exim runtime user, `spool_directory` can't be coerced to be a malicious value unless someone does something especially stupid with macros. Meanwhile, `message_id` is the _Exim_ message-id, such as `1XvGum-000F7a-TQ`, not anything from the received mail. As to `:` vs `true` -- it's a non-issue. Syntactically they're the same, they make no difference to parsing or safety. The only difference between `:` and `true` is that POSIX requires `:` to be a built-in, while `true` is only de-facto built-in. So the issue comes down to "what should be documented as a guide for people to use"; humans not trained in pedantry tend to not treat punctuation as significant and colon vs semi-colon is hard to observe for said non-pedants. In this, I am labelling anyone who is adept in shell as a de-facto pedant. This includes myself. So while I tend to use the >>: ${foo:=default}<< idiom myself, and think colon makes sense for code, I _don't_ think that it makes sense to use it in documentation of walk-through examples. We'll spend too much time correcting peoples' problems where they "slightly" messed up the punctuation. Documentation should be as easy to comprehend as is practical, with the least amount of subtlety in examples as is practical. Thus if we are going to document how to work around return value checking, then I strongly endorse using `true` not `:`. That said: unless there's a reason to break working configurations, we should be extremely reluctant to do so absent a major version bump or a security motivation. We may need to consider either `cmdline-checked:` or `cmdline/options/here:` as syntaxes (syntaces?) and, for "historical compatibility" reasons, default a bare `cmdline:` to "return code ignored". I can't help but think about the `ignore_status` and `temp_errors` options on pipe transports and wonder if there's something there we can use as a naming convention. `cmdline/no_ignore_status:` ? -Phil -- ## List details at https://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##