Patchwork patchbomb: protect email addresses from shell

login
register
mail settings
Submitter Floris Bruynooghe
Date Sept. 29, 2019, 10:09 p.m.
Message ID <01ba660965efded7d336.1569794995@powell.devork.be>
Download mbox | patch
Permalink /patch/41855/
State Superseded
Headers show

Comments

Floris Bruynooghe - Sept. 29, 2019, 10:09 p.m.
# HG changeset patch
# User Floris Bruynooghe <flub@google.com>
# Date 1569794518 -7200
#      Mon Sep 30 00:01:58 2019 +0200
# Node ID 01ba660965efded7d336ecf06270117bf98c6669
# Parent  bbf77341a956b3ba40ae87538fcd200b34c5a3e5
patchbomb: protect email addresses from shell

When patchbomb sends email via a sendmail-like program it invokes this
using procutil.popen which passes the string to a shell to be parsed.
To protect any special characters in the email addresses on the
command line from being interpretered by the shell they must be
quoted.
Augie Fackler - Sept. 30, 2019, 4:21 p.m.
> On Sep 29, 2019, at 18:09, Floris Bruynooghe <flub@devork.be> wrote:
> 
> # HG changeset patch
> # User Floris Bruynooghe <flub@google.com>
> # Date 1569794518 -7200
> #      Mon Sep 30 00:01:58 2019 +0200
> # Node ID 01ba660965efded7d336ecf06270117bf98c6669
> # Parent  bbf77341a956b3ba40ae87538fcd200b34c5a3e5
> patchbomb: protect email addresses from shell
> 
> When patchbomb sends email via a sendmail-like program it invokes this
> using procutil.popen which passes the string to a shell to be parsed.
> To protect any special characters in the email addresses on the
> command line from being interpretered by the shell they must be
> quoted.
> 
> diff --git a/mercurial/mail.py b/mercurial/mail.py
> --- a/mercurial/mail.py
> +++ b/mercurial/mail.py
> @@ -152,7 +152,7 @@ def _smtp(ui):
> def _sendmail(ui, sender, recipients, msg):
>     '''send mail using sendmail.'''
>     program = ui.config('email', 'method')
> -    stremail = lambda x: stringutil.email(encoding.strtolocal(x))
> +    stremail = lambda x: '\'' + stringutil.email(encoding.strtolocal(x)) + '\''

Whee. I suspect we should use shlex.quote() on the the string, rather than blindly inserting ' characters. You'll need to import it like this:

try:
    import shlex
    shellquote = shlex.quote
except (ImportError, AttributeError):
    import pipes
    shellquote = pipes.quote

because Python 2 doesn't have it in the usual module. Could I have you do a resend with that modification?

>     cmdline = '%s -f %s %s' % (program, stremail(sender),
>                                ' '.join(map(stremail, recipients)))
>     ui.note(_('sending mail: %s\n') % cmdline)
> diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
> --- a/tests/test-patchbomb.t
> +++ b/tests/test-patchbomb.t
> @@ -3033,7 +3033,7 @@ single rev
>   +d
> 
>   sending [PATCH] test ...
> -  sending mail: $TESTTMP/t2/pretendmail.sh -f test foo
> +  sending mail: $TESTTMP/t2/pretendmail.sh -f 'test' 'foo'
> 
> Test pull url header
> =================================
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Floris Bruynooghe - Sept. 30, 2019, 7:24 p.m.
On Mon 30 Sep 2019 at 12:21 -0400, Augie Fackler wrote:
>
> Whee. I suspect we should use shlex.quote() on the the string, rather
> than blindly inserting ' characters.

Good catch.  It came with a range of design and style decisions and the
need for more tests.  Hope I choose reasonable for them.

Cheers,
Floris

Patch

diff --git a/mercurial/mail.py b/mercurial/mail.py
--- a/mercurial/mail.py
+++ b/mercurial/mail.py
@@ -152,7 +152,7 @@  def _smtp(ui):
 def _sendmail(ui, sender, recipients, msg):
     '''send mail using sendmail.'''
     program = ui.config('email', 'method')
-    stremail = lambda x: stringutil.email(encoding.strtolocal(x))
+    stremail = lambda x: '\'' + stringutil.email(encoding.strtolocal(x)) + '\''
     cmdline = '%s -f %s %s' % (program, stremail(sender),
                                ' '.join(map(stremail, recipients)))
     ui.note(_('sending mail: %s\n') % cmdline)
diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
--- a/tests/test-patchbomb.t
+++ b/tests/test-patchbomb.t
@@ -3033,7 +3033,7 @@  single rev
   +d
   
   sending [PATCH] test ...
-  sending mail: $TESTTMP/t2/pretendmail.sh -f test foo
+  sending mail: $TESTTMP/t2/pretendmail.sh -f 'test' 'foo'
 
 Test pull url header
 =================================