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 <james...@debian.org>
To: cont...@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-james...@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 <james...@debian.org>
signature.asc
Description: PGP signature
