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 ( "uscanXXXX", 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
Reply to: