Closed
Bug 183188
Opened 22 years ago
Closed 22 years ago
data/mining is world writable after each run of collectstats.pl
Categories
(Bugzilla :: Reporting/Charting, defect)
Bugzilla
Reporting/Charting
Tracking
()
RESOLVED
FIXED
Bugzilla 2.18
People
(Reporter: Franke, Assigned: Franke)
Details
(Whiteboard: [fixed on trunk][fixed in 2.16.2][fixed in 2.14.5])
Attachments
(1 file)
426 bytes,
patch
|
justdave
:
review+
bbaetz
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.2) Gecko/20021126 Build Identifier: data/mining is world writable after each run of collectstats.pl Reproducible: Always Steps to Reproduce: 1. $ cd /somewhere/bugs 2. $ chmod og-w data/mining 3. $ ./collectstats.pl 4. $ ls -ld data/mining drwxrwxrwx ............ data/mining Actual Results: data/mining is world writable after each run of collectstats.pl Expected Results: chmod 0777,data/mining only if dir created by collectstats.pl
Assignee | ||
Comment 1•22 years ago
|
||
Comment 2•22 years ago
|
||
Marking security-sensitive. I will evaluate this tonight. Gerv
Group: webtools-security
Comment 3•22 years ago
|
||
Christian: do you have a value set for $webservergroup in localconfig? If you don't, that's the intended behaviour, because that directory had to be world-writable for both the cron job and the webserver to get at it, since they run as different users. If you are using a webservergroup, then this is not as bad as it sounds, because the data directory itself should not be world-readable, so the world-whatever access on anything inside it is not quite so relevant. You're still only going to have access to it from people who are also in the webservergroup, which is going to have write access to it anyway. My opinion is this is not a security issue because of the above.
Comment 4•22 years ago
|
||
OK, I take that back, with a webservergroup, the data directory is going to be chmod 771, which does make it world-accessible (but not world-readable), which means anything world-readable inside it will be accessible if they know the filename, so yes this is an issue, if it's still doing that with $webservergroup set.
Comment 5•22 years ago
|
||
confirmed on landfill. collectstats.pl is ignoring $webservergroup, and making it world-writable regardless.
Severity: normal → critical
Target Milestone: --- → Bugzilla 2.18
Comment 6•22 years ago
|
||
collectstats does (for each product) sub check_data_dir { my $dir = shift; if (! -d) { mkdir $dir, 0777; chmod 0777, $dir; } } with an arg of "data/mining". This shouldn't be needed, because checketup sets this stuff up, but since it is a directory this code shouldn't be firing anyway. We should jus remove this check, but why is teh chmod being hit at all? (Theres no other chmod call in that file)
Comment 7•22 years ago
|
||
my $dir = shift Doesn't that remove the $_ that was the original argument to the sub that -d will act on if it there's no argument to -d? That test probably needs to be if (! -d $dir) {
Assignee | ||
Comment 8•22 years ago
|
||
Dave: I agree with comment 7. Look at my patch attached in comment 1, this is exactly the change which works for us ;-) This bug may not be visible on all platforms: Depending on perl and/or OS version, the expression (! -d) (same as (! -d $_) always and (! -d "") here) may be always true, because the empty string may be considered equivalent to ".". I don't know for sure. Bradley: I agree with comment 6. collectstat.pl should simply die if (! -d "data/mining"); Only checksetup.pl should handle directory creation and permissions.
Comment 9•22 years ago
|
||
Yeah, I thought we pulled all the mkdir stuff a while back - must have missed one
Comment 10•22 years ago
|
||
From Comment #2 (Gervase Markham 2002-12-03 04:11) > Marking security-sensitive. I will evaluate this tonight. ping?
Comment 11•22 years ago
|
||
Well, it looked like you guys have already done the evaluation :-) Gerv
Comment 12•22 years ago
|
||
Comment on attachment 108014 [details] [diff] [review] Patch to collectstats.pl which WORKSFORME ;-) while this doesn't remove the mkdir, it does seem to solve the security issue....
Attachment #108014 -
Flags: review+
Updated•22 years ago
|
Attachment #108014 -
Flags: review?(bbaetz)
Comment 13•22 years ago
|
||
Comment on attachment 108014 [details] [diff] [review] Patch to collectstats.pl which WORKSFORME ;-) yeah, ok, r=bbaetz
Attachment #108014 -
Flags: review?(bbaetz)
Comment 15•22 years ago
|
||
checked in on trunk: Checking in collectstats.pl; /cvsroot/mozilla/webtools/bugzilla/collectstats.pl,v <-- collectstats.pl new revision: 1.24; previous revision: 1.23 done checked in on BUGZILLA-2_16-BRANCH: Checking in collectstats.pl; /cvsroot/mozilla/webtools/bugzilla/collectstats.pl,v <-- collectstats.pl new revision: 1.20.12.1; previous revision: 1.20 done checked in on BUGZILLA-2_14_1-BRANCH: Checking in collectstats.pl; /cvsroot/mozilla/webtools/bugzilla/collectstats.pl,v <-- collectstats.pl new revision: 1.20.10.1; previous revision: 1.20 done same patch applied cleanly to all three.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Whiteboard: [fixed on trunk][fixed in 2.16.2][fixed in 2.14.5]
Comment 16•22 years ago
|
||
public notice has been made, clearing security flag
Group: webtools-security
Updated•12 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•