Skip to content

Commit

Permalink
Bug 1079065: [SECURITY] Always use the 3 arguments form for open() to…
Browse files Browse the repository at this point in the history
… prevent shell code injection

r=dylan,a=simon
  • Loading branch information
gerv authored and dklawren committed Jan 21, 2015
1 parent d98f8cd commit f5b9cba
Show file tree
Hide file tree
Showing 12 changed files with 18 additions and 18 deletions.
4 changes: 2 additions & 2 deletions Bugzilla/Attachment.pm
Expand Up @@ -342,7 +342,7 @@ sub data {
# If there's no attachment data in the database, the attachment is stored
# in a local file, so retrieve it from there.
if (length($self->{data}) == 0) {
if (open(AH, $self->_get_local_filename())) {
if (open(AH, '<', $self->_get_local_filename())) {
local $/;
binmode AH;
$self->{data} = <AH>;
Expand Down Expand Up @@ -388,7 +388,7 @@ sub datasize {
# is stored in a local file, and so retrieve its size from the file,
# or the attachment has been deleted.
unless ($self->{datasize}) {
if (open(AH, $self->_get_local_filename())) {
if (open(AH, '<', $self->_get_local_filename())) {
binmode AH;
$self->{datasize} = (stat(AH))[7];
close(AH);
Expand Down
2 changes: 1 addition & 1 deletion Bugzilla/Attachment/PatchReader.pm
Expand Up @@ -99,7 +99,7 @@ sub process_interdiff {
# Send through interdiff, send output directly to template.
# Must hack path so that interdiff will work.
$ENV{'PATH'} = $lc->{diffpath};
open my $interdiff_fh, "$lc->{interdiffbin} $old_filename $new_filename|";
open my $interdiff_fh, '-|', "$lc->{interdiffbin} $old_filename $new_filename";
binmode $interdiff_fh;
my ($reader, $last_reader) = setup_patch_readers("", $context);

Expand Down
2 changes: 1 addition & 1 deletion Bugzilla/Config/Common.pm
Expand Up @@ -231,7 +231,7 @@ sub check_webdotbase {
# Check .htaccess allows access to generated images
my $webdotdir = bz_locations()->{'webdotdir'};
if(-e "$webdotdir/.htaccess") {
open HTACCESS, "$webdotdir/.htaccess";
open HTACCESS, "<", "$webdotdir/.htaccess";
if(! grep(/ \\\.png\$/,<HTACCESS>)) {
return "Dependency graph images are not accessible.\nAssuming that you have not modified the file, delete $webdotdir/.htaccess and re-run checksetup.pl to rectify.\n";
}
Expand Down
2 changes: 1 addition & 1 deletion Bugzilla/Error.pm
Expand Up @@ -71,7 +71,7 @@ sub _throw_error {
$val = "*****" if $val =~ /password|http_pass/i;
$mesg .= "[$$] " . Data::Dumper->Dump([$val],["env($var)"]);
}
open(ERRORLOGFID, ">>$datadir/errorlog");
open(ERRORLOGFID, ">>", "$datadir/errorlog");
print ERRORLOGFID "$mesg\n";
close ERRORLOGFID;
}
Expand Down
4 changes: 2 additions & 2 deletions Bugzilla/Install/CPAN.pm
Expand Up @@ -203,8 +203,8 @@ sub set_cpan_config {
# Calling a senseless autoload that does nothing makes us
# automatically load any existing configuration.
# We want to avoid the "invalid command" message.
open(my $saveout, ">&STDOUT");
open(STDOUT, '>/dev/null');
open(my $saveout, ">&", "STDOUT");
open(STDOUT, '>', '/dev/null');
eval { CPAN->ignore_this_error_message_from_bugzilla; };
undef $@;
close(STDOUT);
Expand Down
2 changes: 1 addition & 1 deletion Bugzilla/Install/Filesystem.pm
Expand Up @@ -574,7 +574,7 @@ sub _update_old_charts {
($in_file =~ /\.orig$/i));

rename("$in_file", "$in_file.orig") or next;
open(IN, "$in_file.orig") or next;
open(IN, "<", "$in_file.orig") or next;
open(OUT, '>', $in_file) or next;

# Fields in the header
Expand Down
2 changes: 1 addition & 1 deletion Bugzilla/Send/Sendmail.pm
Expand Up @@ -29,7 +29,7 @@ sub send {

my $pipe = gensym;

open($pipe, "| $mailer -t -oi @args")
open($pipe, "|-", "$mailer -t -oi @args")
|| return failure "Error executing $mailer: $!";
print($pipe $message->as_string)
|| return failure "Error printing via pipe to $mailer: $!";
Expand Down
2 changes: 1 addition & 1 deletion collectstats.pl
Expand Up @@ -321,7 +321,7 @@ sub regenerate_stats {
return;
}

if (open DATA, ">$file") {
if (open DATA, ">", $file) {
my $fields = join('|', ('DATE', @statuses, @resolutions));
print DATA <<FIN;
# Bugzilla Daily Bug Stats
Expand Down
2 changes: 1 addition & 1 deletion reports.cgi
Expand Up @@ -136,7 +136,7 @@ sub generate_chart {
$data_file =~ s/\//-/gs;
$data_file = $dir . '/' . $data_file;

if (! open FILE, $data_file) {
if (!open(FILE, '<', $data_file)) {
if ($product eq '-All-') {
$product = '';
}
Expand Down
2 changes: 1 addition & 1 deletion search_plugin.cgi
Expand Up @@ -24,7 +24,7 @@ print $cgi->header('application/xml');

# Get the contents of favicon.ico
my $filename = bz_locations()->{'libpath'} . "/images/favicon.ico";
if (open(IN, $filename)) {
if (open(IN, '<', $filename)) {
local $/;
binmode IN;
$vars->{'favicon'} = <IN>;
Expand Down
6 changes: 3 additions & 3 deletions showdependencygraph.cgi
Expand Up @@ -46,7 +46,7 @@ sub CreateImagemap {
my $map = "<map name=\"imagemap\">\n";
my $default = "";

open MAP, "<$mapfilename";
open MAP, "<", $mapfilename;
while(my $line = <MAP>) {
if($line =~ /^default ([^ ]*)(.*)$/) {
$default = qq{<area alt="" shape="default" href="$1">\n};
Expand Down Expand Up @@ -247,7 +247,7 @@ if ($webdotbase =~ /^https?:/) {
error => $! });

binmode $pngfh;
open(DOT, "\"$webdotbase\" -Tpng $filename|");
open(DOT, '-|', "\"$webdotbase\" -Tpng $filename");
binmode DOT;
print $pngfh $_ while <DOT>;
close DOT;
Expand Down Expand Up @@ -276,7 +276,7 @@ if ($webdotbase =~ /^https?:/) {
error => $! });

binmode $mapfh;
open(DOT, "\"$webdotbase\" -Tismap $filename|");
open(DOT, '-|', "\"$webdotbase\" -Tismap $filename");
binmode DOT;
print $mapfh $_ while <DOT>;
close DOT;
Expand Down
6 changes: 3 additions & 3 deletions testserver.pl
Expand Up @@ -37,7 +37,7 @@
my $sgid = 0;
if (!ON_WINDOWS) {
foreach my $pscmd (@pscmds) {
open PH, "$pscmd 2>/dev/null |";
open PH, '-|', "$pscmd 2>/dev/null";
while (my $line = <PH>) {
if ($line =~ /^(?:\S*\/)?(?:httpd|apache?)2?\s+(\d+)$/) {
$sgid = $1 if $1 > $sgid;
Expand Down Expand Up @@ -264,7 +264,7 @@ sub check_image {

sub create_file {
my ($filename, $content) = @_;
open(FH, ">$filename")
open(FH, ">", $filename)
or die "Failed to create $filename: $!\n";
binmode FH;
print FH $content;
Expand All @@ -273,7 +273,7 @@ sub create_file {

sub read_file {
my ($filename) = @_;
open(FH, $filename)
open(FH, '<', $filename)
or die "Failed to open $filename: $!\n";
binmode FH;
my $content = <FH>;
Expand Down

0 comments on commit f5b9cba

Please sign in to comment.