Re: [Nmh-workers] proposed patch for shell metacharacter failure in nmh-1.7

[David Levine]

Ken wrote:

> I really think to be safe we should simply
> replace any shell metacharacters for those things,

I'm not sure that could be done completely safely.  See below about not
using /bin/sh -c.

> because I can imagine
> some nasty security holes that we might encounter.

The current master and 1.7 branch have a security hole depending on what
the user puts in their profile (%{name}, etc), see example below.

Paul V wrote:

> must we call /bin/sh -c "$foo", or can we call execve on the command 
> itself, after cracking it into an argv[] ?

argsplit uses /bin/sh -c by design:

 * - If we have shell metacharacters, run the command using
 *   /bin/sh -c 'command "$@"'.  In this case, any additional arguments
 *   appended to the arglist will be expanded by "$@".

I agree that we should revisit this.

David


$ cat MH
#: Path not shown
mhshow-show-application/pdf: %pecho %{name}

$ cat `mhpath +inbox 51`
Subject: shows danger of current quoting + /bin/sh -c
MIME-Version: 1.0
Content-Type: application/pdf; name="oops'; /bin/rm foo"
Date: Mon, 15 Jan 2018 08:00:00 -0500
Message-ID: <123456>

$ MH=MH mhshow -noinline -notext +inbox 51
[ Message inbox:51 ]
Date:    Mon, 15 Jan 2018 08:00:00 -0500
Subject: shows danger of current quoting + /bin/sh -c

MIME-Version: 1.0

[ part  - application/pdf - oops'; /bin/rm foo  0B  ]
oops'
/bin/rm: cannot remove 'foo': No such file or directory
echo 'oops'\'; /bin/rm foo "$@": exited 1