Re: [PATCH] cvsimport: rewrite to use cvsps 3.x to fix major bugs

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 11, 2013, 19:17 UTC
Message-ID: <7vr4lrbkcs.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130111185818.GA30398@thyrsus.com>

"Eric S. Raymond" <esr@thyrsus.com> writes:

Show 14 quoted lines

> Junio C Hamano <gitster@pobox.com>:
>> I think the prevalent style in this script is to write "print"
>> without parentheses:
>> 
>> 	print STDERR "msg\n";
>
> That can be easily fixed.
>
>> This looks lazy and unsafe quoting.  Is there anything that makes
>> sure repository path does not contain a single quote?
>
> No. But...wait, checking...the Perl code didn't have the analogous
> check, so there's no increased vulnerability here.  I'll put it on the
> to-do list for after I ship parsecvs.

I checked before I sent that review, and as far as I could tell, it was fairly consistently avoiding the lazy and insecure forms, e.g.

	system("com mand " . $param);
	open($fh, "com mand " . $param . " |"); while (<$fh>)	{ ... }

but used the more sequre list form, e.g.

	system(qw(com mand), $param);
        open($fh, "-|", qw(com mand), $param); while (<$fh>)	{ ... }

But of course there may be some places that were careless that I didn't spot (and previous reviewers of the current cvsimport didn't).