Re: Files-Excluded field and security implications of uscan and debian/copyright.
On Tue, 11 Sep 2012 22:45:07 +0200, Andreas Tille wrote: On Tue, Sep 11, 2012 at 07:11:20PM +0200, gregor herrmann wrote: like calls because system does not return the number of files. I'm attaching a small example that uses File::Find for this purpose. Do I understand you correctly that these are just academic examples to spread some Perl knowledge (which is perfectly welcome) or do you want me to apply this instead of the `` call? To be honest: I didn't see any actual problems with your current backtick solution, just wanted to try to implement it in Perl. So unless Don or someone else more experienced finds a problem, there's probably no need to switch. Cheers, gregor -- .''`. Homepage: http://info.comodo.priv.at/ - OpenPGP key 0xBB3A68018649AA06 : :' : Debian GNU/Linux user, admin, and developer - http://www.debian.org/ `. `' Member of VIBE!AT SPI, fellow of the Free Software Foundation Europe `- NP: Donovan: Till I see you again signature.asc Description: Digital signature
Re: Files-Excluded field and security implications of uscan and debian/copyright.
On Mon, Sep 10, 2012 at 10:07:40AM -0700, Don Armstrong wrote:
lines like the following:
`find $main_source_dir -path $main_source_dir/$_ -print0 | xargs -0 rm
-rf`;
should really be written like this:
system('find',$main_source_dir,'-path',$main_source_dir/$_,qw(-exec rm
-rf {} ;))==0 or
die failure to run find properly;
Done (also added '-depth' parameter to find to enable removing
directories recursively).
Doing the first will cause problems if Files-Excluded: contains an
entry with ,[1] whereas it will be just fine if there aren't any
entries. [You also probably really wanted xargs -0r, just in case
nothing was matched.]
Ditto for everywhere else that backticks is used. [In general, if
you're accepting any user input into a function which calls backticks,
you almost certainly want system() instead. If you want the output of
the command, use three argument open.]
Point taken for those calls where user-input (= strings mentioned in
debian/copyright Files-Excluded) is involved. I left calls like
my $tempdir = tempdir ( uscan, TMPDIR = 1, CLEANUP = 1 );
my $nfiles_before = `find $tempdir | wc -l`;
like calls because system does not return the number of files. Strictly
speaking the two lines above are a shortcut and in the code the dir
which is seeked by find is mangled which might or might not involve the
name of the directory in the upstream archive. So if somebody considers
some upstream naming its source dir something like bla; rm -rf ~ with
appropriate quoting as a probable intrusion vector I would welcome some
help to even more safely count the number of files in a given directory.
(You could also avoid calling out to find completely, and use
Find::File and File::Path::rmtree or similar, but that's a more
personal decision.)
I'm fine with anything that works - my method was the first one that
came to mind. I have no idea in how far system('find',...) compares to
Find::File and in how for this difference is relevant for the intended
purpose.
1: I haven't checked to see whether could even make it through to
the backticks code, but it's better to just handle it properly in the
first place.
ACK.
Thanks for the hint
Andreas.
--
http://fam-tille.de
--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20120911155444.gf14...@an3as.eu
Re: Files-Excluded field and security implications of uscan and debian/copyright.
On Tue, 11 Sep 2012 17:54:44 +0200, Andreas Tille wrote:
Point taken for those calls where user-input (= strings mentioned in
debian/copyright Files-Excluded) is involved. I left calls like
my $tempdir = tempdir ( uscan, TMPDIR = 1, CLEANUP = 1 );
my $nfiles_before = `find $tempdir | wc -l`;
like calls because system does not return the number of files.
I'm attaching a small example that uses File::Find for this purpose.
(You could also avoid calling out to find completely, and use
Find::File and File::Path::rmtree or similar, but that's a more
personal decision.)
I'm fine with anything that works - my method was the first one that
came to mind. I have no idea in how far system('find',...) compares to
Find::File and in how for this difference is relevant for the intended
purpose.
When I looked at your patches first, I also wanted to suggest to
replace various calls of system() by perl modules [0] -- but then I saw
that system() is used all over the place in uscan.pl, so that would
mean a major rewrite ...
Cheers,
gregor
[0] e.g. Archive::Tar or Archive::Zip
--
.''`. Homepage: http://info.comodo.priv.at/ - OpenPGP key 0xBB3A68018649AA06
: :' : Debian GNU/Linux user, admin, and developer - http://www.debian.org/
`. `' Member of VIBE!AT SPI, fellow of the Free Software Foundation Europe
`- NP: Don McLean: Crying
#!/usr/bin/perl
use strict;
use warnings;
use File::Temp qw/ tempdir /;
use File::Find;
my $tempdir = tempdir( uscan, TMPDIR = 1, CLEANUP = 1 );
# create some files for testing
system( 'mkdir', $tempdir/a, $tempdir/b );
system( 'touch', $tempdir/a/1, $tempdir/b/1 );
my $nfiles_before = 0;
# system/find/wc
chomp( $nfiles_before = `find $tempdir | wc -l` );
print find found $nfiles_before files.\n;
$nfiles_before = 0;
# File::Find
find( \countfiles, $tempdir );
sub countfiles { $nfiles_before++; }
print File::Find found $nfiles_before files.\n;
signature.asc
Description: Digital signature
Re: Files-Excluded field and security implications of uscan and debian/copyright.
Hi Gregor, On Tue, Sep 11, 2012 at 07:11:20PM +0200, gregor herrmann wrote: like calls because system does not return the number of files. I'm attaching a small example that uses File::Find for this purpose. Do I understand you correctly that these are just academic examples to spread some Perl knowledge (which is perfectly welcome) or do you want me to apply this instead of the `` call? When I looked at your patches first, I also wanted to suggest to replace various calls of system() by perl modules [0] -- but then I saw that system() is used all over the place in uscan.pl, so that would mean a major rewrite ... Yep. I think for the intended purpose of uscan the use of system is appropriate and a rewrite makes no real sense. Kind regards Andreas. -- http://fam-tille.de -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20120911204507.gh14...@an3as.eu
Re: Files-Excluded field and security implications of uscan and debian/copyright.
Hi Charles, On Mon, Sep 10, 2012 at 08:20:43AM +0900, Charles Plessy wrote: I would love to get a pointer to the actual line[1] which executes content from debian/copyright. TTBOMK, all expressions are part of the seeking string of a find statement, nothing more. the find commands are executed via backsticks, which potentially can execute any arbitrary command. I personally have not found a way to exploit this (*), but given my lack of training in the field, I do not consider this significant, so I asked for others opinions. But these are totally different things: I understood your initial mail that using debian/copyright is insecure. Now you come up with the argument that using backsticks might be insecure. So either backsticks are insecure for *any* file we are using (IMHO the current implementation is not - but Perl experts might have another look at[1]) or not. My main question anyway is whether it would be useful to make a distinction between fields that have a content that is more likely to be passed to shell commands, and fields where the content is less likely to be so. I try to give a short history of the proposal to get a reasonably option of removing files from upstream source in an easy manner: My first implementation suggestion[2] was to use a file uscan.remove which was enhanced by the IMHO very sane recommendation of Jonas[3] to use debian/copyright. We had some opinion raised against this (none of them contained a security argument and IMHO this argument is void) but from my perspective there is some kind of consensus that using debian/copyright is a good idea - at least this is my personal reading of the threads. We now could either try to get a proof of this consensus by some kind of voting or we could follow an implementation (Do-O-Cracy). For the sake of simplicity I would prefer the later way to step forewars and I'm perfectly fine for accepting patches to[1] which solves any issue anybody might have to the current implementation suggestion. (*) Yes I looked, and maybe the most straightforward way would be to make a fake file name containing backsticks, in order to execute a helper script in the debian directory. Huh what? A Debian developer who tries to delete an upstream file that contains backsticks in the file name and contains vulnerable code by injecting it into debian/copryight? Are you honest that this could be a reasonable way of attacking a Debian system? Any get-orig-source target might be way more dangerous than this - and it is the Debian developers duty anyway to check the result of the tarball creation process. If any upstream (and we are talking about files in an upstream tarball, right) wants to attack a Debian system - why should he put the code in this very crude way into the tarball and not straight into the code??? Kind regards Andreas. [1] http://anonscm.debian.org/gitweb/?p=users/tille/devscripts.git;a=blob;f=scripts/uscan.pl;hb=HEAD [2] http://lists.debian.org/debian-devel/2012/08/msg00397.html [3] http://lists.debian.org/debian-devel/2012/08/msg00406.html -- http://fam-tille.de -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20120910071817.gc16...@an3as.eu
Re: Files-Excluded field and security implications of uscan and debian/copyright.
On Mon, 10 Sep 2012, Andreas Tille wrote:
But these are totally different things: I understood your initial
mail that using debian/copyright is insecure. Now you come up with
the argument that using backsticks might be insecure. So either
backsticks are insecure for *any* file we are using (IMHO the
current implementation is not - but Perl experts might have another
look at[1]) or not.
lines like the following:
`find $main_source_dir -path $main_source_dir/$_ -print0 | xargs -0 rm
-rf`;
should really be written like this:
system('find',$main_source_dir,'-path',$main_source_dir/$_,qw(-exec rm -rf
{} ;))==0 or
die failure to run find properly;
Doing the first will cause problems if Files-Excluded: contains an
entry with ,[1] whereas it will be just fine if there aren't any
entries. [You also probably really wanted xargs -0r, just in case
nothing was matched.]
Ditto for everywhere else that backticks is used. [In general, if
you're accepting any user input into a function which calls backticks,
you almost certainly want system() instead. If you want the output of
the command, use three argument open.]
(You could also avoid calling out to find completely, and use
Find::File and File::Path::rmtree or similar, but that's a more
personal decision.)
Don Armstrong
1: I haven't checked to see whether could even make it through to
the backticks code, but it's better to just handle it properly in the
first place.
--
I don't care how poor and inefficient a little country is; they like
to run their own business. I know men that would make my wife a
better husband than I am; but, darn it, I'm not going to give her to
'em.
-- The Best of Will Rogers
http://www.donarmstrong.com http://rzlab.ucr.edu
--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20120910170740.gs8...@rzlab.ucr.edu
Re: Files-Excluded field and security implications of uscan and debian/copyright.
On Fri, Sep 07, 2012 at 03:15:27PM +0100, Ian Jackson wrote: Charles Plessy writes (Re: Files-Excluded field and security implications of uscan and debian/copyright.): Le Fri, Sep 07, 2012 at 08:44:36AM +0900, Charles Plessy a écrit : in the case of the Files-Excluded field, the contents of the field are directly executed. I mean: the contents are transferred to an expression that is directly executed. This is a bug in the implementations that do that, surely ? ??? I would love to get a pointer to the actual line[1] which executes content from debian/copyright. TTBOMK, all expressions are part of the seeking string of a find statement, nothing more. Kind regards Andreas. [1] http://anonscm.debian.org/gitweb/?p=users/tille/devscripts.git;a=blob;f=scripts/uscan.pl;hb=HEAD -- http://fam-tille.de -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20120909210444.gb3...@an3as.eu
Re: Files-Excluded field and security implications of uscan and debian/copyright.
Le Sun, Sep 09, 2012 at 11:04:44PM +0200, Andreas Tille a écrit : On Fri, Sep 07, 2012 at 03:15:27PM +0100, Ian Jackson wrote: Charles Plessy writes (Re: Files-Excluded field and security implications of uscan and debian/copyright.): Le Fri, Sep 07, 2012 at 08:44:36AM +0900, Charles Plessy a écrit : in the case of the Files-Excluded field, the contents of the field are directly executed. I mean: the contents are transferred to an expression that is directly executed. This is a bug in the implementations that do that, surely ? ??? I would love to get a pointer to the actual line[1] which executes content from debian/copyright. TTBOMK, all expressions are part of the seeking string of a find statement, nothing more. Hi Andreas, the find commands are executed via backsticks, which potentially can execute any arbitrary command. I personally have not found a way to exploit this (*), but given my lack of training in the field, I do not consider this significant, so I asked for others opinions. My main question anyway is whether it would be useful to make a distinction between fields that have a content that is more likely to be passed to shell commands, and fields where the content is less likely to be so. (*) Yes I looked, and maybe the most straightforward way would be to make a fake file name containing backsticks, in order to execute a helper script in the debian directory. Have a nice day, -- Charles Plessy Tsurumi, Kanagawa, Japan -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20120909232043.ga32...@falafel.plessy.net
Re: Files-Excluded field and security implications of uscan and debian/copyright.
Charles Plessy writes (Re: Files-Excluded field and security implications of uscan and debian/copyright.): Le Fri, Sep 07, 2012 at 08:44:36AM +0900, Charles Plessy a écrit : in the case of the Files-Excluded field, the contents of the field are directly executed. I mean: the contents are transferred to an expression that is directly executed. This is a bug in the implementations that do that, surely ? Ian. -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20554.383.720037.437...@chiark.greenend.org.uk
Files-Excluded field and security implications of uscan and debian/copyright.
Hi Andreas and everybody, while drafting the IANA registration for the machine-readable Debian copyright format, I had to consider and describe security implications, and realised that in the case of the Files-Excluded field, the contents of the field are directly executed. One can imagine scenarios where an attacker inserts in that field some code that will cause uscan to send the developer's GPG or SSH private keys to a remote address, etc. This can of course be corrected in uscan, but it leads me to wonder if the copyright file should be kept as declarative as possible. This would mean that the fileds that are not only informative, but that are also intended to control how a program is executed, like Files-Excluded, would rather be placed in more specialised configuration files. For instance, one could think a format revision 4 for debian/watch, that would use exactly the syntax that you are implementing now, and that would embed the watch information in a Watch field. This would not completely solve security issues for debian/copyright, as one can also imagine that some programs looking for files in the Files field may also pass the values witout sanitisation to some shell commands. However, I wonder if it would not be better to keep it as declarative as possible. What do you think ? -- Charles Plessy Tsurumi, Kanagawa, Japan -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20120906234436.ga7...@falafel.plessy.net
Re: Files-Excluded field and security implications of uscan and debian/copyright.
Le Fri, Sep 07, 2012 at 08:44:36AM +0900, Charles Plessy a écrit : in the case of the Files-Excluded field, the contents of the field are directly executed. I mean: the contents are transferred to an expression that is directly executed. Sorry for the noise, -- Charles -- To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20120907041146.gc21...@falafel.plessy.net
