Bug#809318: bts: overrides user-specified value of sendmail

[James McCoy]

On Tue, Dec 29, 2015 at 10:15:15AM +0000, Daniel Shahaf wrote:
> bts(1) sent an email without my permission:
> ..
>     % bts --sendmail='() { cat $1 > /dev/tty }' reopen 999999 
>     --sendmail command contained funny characters: ()
>     Reverting to default value /usr/sbin/sendmail
>     %
> ..
> 
> I expected it to invoke «system('() { cat $1 > /dev/tty } /path/to/file')»¹,
> which would have printed the email to /dev/tty without sending it.

FWIW, the -n option could be useful here. ☺

    % bts -n reopen 999999
    From: James McCoy <jamessan at debian.org>
    To: control at bugs.debian.org
    Subject: reopening 999999
    Date: Tue, 29 Dec 2015 08:31:25 -0500
    User-Agent: devscripts bts/2.15.9
    Message-ID: <1451395885-805-bts-jamessan at debian.org>

    reopen 999999
    thanks

> Personally, I don't see why bts(1) validates the user-specified value:

It's been like that since the functionality was first introduced.

> there's no trust boundary here so there's no need to guard for shell
> injections.  That said, if validation is done and fails, bts(1) should
> simply error out hard.

On first blush, I'd agree with this but I'm open to feedback from
others.

> Also, the patch doesn't cause system() to be invoked on the argument
> value; the value is split on spaces and fed to exec(), which fails with
> «Can't exec "()": No such file or directory at scripts/bts.pl line 2651.».

Hmm, we should probably be using Text::ParseWords' shellwords function
instead.

Cheers,
-- 
James
GPG Key: 4096R/331BA3DB 2011-12-05 James McCoy <jamessan at debian.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 949 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/devscripts-devel/attachments/20151229/aef7efb5/attachment.sig>