Re: [PATCH] avoid insecure use of mail in man page example

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 28, 2021, 23:46 UTC
Message-ID: <xmqqtui4gt5f.fsf@gitster.g>
In-Reply-To: <YVNi91WYyj3Le6UF@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 22 quoted lines

> On Tue, Sep 28, 2021 at 08:16:48AM -0400, Joey Hess wrote:
>
>> As recently seen in fail2ban's security hole (CVE-2021-32749),
>> piping user controlled input to mail is exploitable,
>> since a line starting with "~! foo" in the input will run command foo.
>> 
>> This example on the man page pipes to mail. It may not be exploitable.
>> git rev-list --pretty indents commit messages, which prevents the escape
>> sequence working there. It's less clear if it might be possible to embed
>> the escape sequence in a signed push certificate. The user reading the
>> man page might alter the example to do something more exploitable.
>> To encourage safe use of mail, add -E 'set escape'
>
> Seems like a good goal, but is "-E" portable?
>
> On my system, where "mail" comes from the bsd-mailx package, "-E" means
> "do not send a message with an empty body" and your example command
> barfs as it tries to deliver to the recipient "set escape".
>
> At least we'd want to make a note in the documentation saying what the
> mysterious "set escape" is doing, and that not all versions of mail
> would need / want it.

It is not the primary focus for this documentation page to teach how to send e-mails in the first place. Instead of risking confused users rightly complain with "my 'mail' does not understand the -E option---what does this do?", I wonder if it is better to just change it to

	git rev-list --pretty ...
-   fi |
-   mail -s ...    
+   fi >>/var/log/update.log

so that it illustrates what's available *out* *of* *us* to the authors of the script, without having to teach them "mail" and other things we are responsible for.