Closed Bug 155861 Opened 21 years ago Closed 21 years ago

showdependancygraph.cgi fails taint check with local dot installation

Categories

(Bugzilla :: Reporting/Charting, defect)

2.16
x86
FreeBSD
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 2.16

People

(Reporter: carl, Assigned: bbaetz)

Details

Attachments

(1 file)

Trying to show the dependancy graph using a locally installed version of dot
causes perl to throw a taint "insecure dependancy in showdependancygraph.cgi at
line 192" error.

This line is;

foreach my $f (glob("data/webdot/*.dot 
                     data/webdot/*.png 
                     data/webdot/*.map"))

There is a "trick_taint" further down in the loop which I guess is supposed to
stop this, but it seems to be the glob that is causing it.

I've got it working by replacing the above 3 lines with the following 4;

opendir(DIR, "data/webdot/");
my @taintf = grep { /\.dot$|\.png$|\.map$/ } readdir(DIR);
closedir(DIR);
foreach my $f (@taintf)


I'm not 100% sure the regex is right, but it seems to work now and the graph is
generated.  I'll have to wait a few days to so to see if it is deleting old graphs.

This is in Bugzilla v2.16RC2 running under FreeBSD 4.3-STABLE with Perl 5.005_03.
-> 2.16

just calling glob('*.foo') is insecure in 5.0055, because it uses csh to do the
expansion - 5.8.0RC2 adds to teh perlsec man page:

# In Perl releases older than 5.6.0 the <*.c> and glob('*.c') would
# have used an external program to do the filename expansion; but in
# either case the result is tainted since the list of filenames comes
# from outside of the program.

We need to use readidr, but when deleteing, we also need to prepend 'data/graphs'

-> 2.16
Target Milestone: --- → Bugzilla 2.16
oops missed that.

So we need to add something like this;


    my $pathf = "data/webdot/" . $f;
    if (ModTime($pathf) < $since) {
        unlink $pathf;
    }

at line 209.
Attached patch v1Splinter Review
Before I could test this I needed to get the perms set right. This changes
checksetup.pl to do that.

I don't have access to a perl5.005 system to test this on, since landfill is
down.
-> patch author (me)
Assignee: gerv → bbaetz
Keywords: patch, review
Comment on attachment 90452 [details] [diff] [review]
v1

r=gerv.

Gerv
Attachment #90452 - Flags: review+
Comment on attachment 90452 [details] [diff] [review]
v1

Works, looks-good.  r=myk

Am I right in thinking that mimedump-tmp is used only by contrib scripts and
shouldn't be created by checksetup.pl (and that's why you removed its creation
from the script)?
Attachment #90452 - Flags: review+
I grepped for uses of that dir, but forgot to grep contrib. I've left that line
in, but because of bug 156568 its never actually run, anyway.

I don't have a perl 5.005 bz installation, but I did test this snippit on
solaris 5.005_02 without problems

Checked into trunk + branch

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.160; previous revision: 1.159
done
Checking in showdependencygraph.cgi;
/cvsroot/mozilla/webtools/bugzilla/showdependencygraph.cgi,v  <-- 
showdependencygraph.cgi
new revision: 1.20; previous revision: 1.19
done

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.149.2.8; previous revision: 1.149.2.7
done
Checking in showdependencygraph.cgi;
/cvsroot/mozilla/webtools/bugzilla/showdependencygraph.cgi,v  <-- 
showdependencygraph.cgi
new revision: 1.18.2.1; previous revision: 1.18
done
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.