Checksetup leaves editor backups of localconfig accessible

RESOLVED FIXED in Bugzilla 2.18

Status

()

Bugzilla
Installation & Upgrading
P1
blocker
RESOLVED FIXED
15 years ago
10 months ago

People

(Reporter: Joel Peshkin, Assigned: Joel Peshkin)

Tracking

2.17.1
Bugzilla 2.18
Bug Flags:
approval +

Details

(Whiteboard: [fixed on trunk][fixed in 2.14.5][fixed in 2.16.2])

Attachments

(3 attachments, 7 obsolete attachments)

(Assignee)

Description

15 years ago
Any user who uses vim will have files such as localconfig~ which are left
accessible.
I just fixed this for bugzilla-stable on landfill...

We _so_ need to move all of this stuff out of the webtree, but I don't think we
have the infrastructure yet for that.
(Assignee)

Comment 2

15 years ago
Created attachment 109889 [details] [diff] [review]
Patch for new sites


This is a change to checksetup that causes it to write the right .htaccess file
in the first place

The problem, however, is that checksetup will only create the file if it
doesn't already exist.
(Assignee)

Comment 3

15 years ago
Actually, this should be done as a checksetup issue...
Assignee: justdave → zach
Component: Bugzilla-General → Installation & Upgrading
Priority: -- → P1
Target Milestone: --- → Bugzilla 2.18
(Assignee)

Comment 4

15 years ago
Created attachment 109897 [details] [diff] [review]
Patch including repair code


OK, this patch changes the .htaccess file from 
localconfig|nextfile
to
localconfig.*|nextfile
Which will work for anyone who let checksetup build .htaccess for them and runs
checksetup again.
Attachment #109889 - Attachment is obsolete: true
(Assignee)

Comment 5

15 years ago
Created attachment 109899 [details] [diff] [review]
Same patch for 2_16
(Assignee)

Comment 6

15 years ago
Created attachment 109900 [details] [diff] [review]
Same patch for 2_14

Comment 7

15 years ago
It's a good thing I'm such a slacker and we haven't released yet... ;-)

Are we gonna fold this fix into the security announcement for 2.14.x/2.16.x, and
the three releases due out this weekend (2.14.5 and 2.16.2 and 2.17.2)? Justdave?

Bueller?

Bueller?
yes, we'll hold release for this.

Updated

15 years ago
Attachment #109897 - Flags: review+

Updated

15 years ago
Attachment #109899 - Flags: review+

Updated

15 years ago
Attachment #109900 - Flags: review+
make it so
Assignee: zach → bugreport
Flags: approval+
Whiteboard: [wanted for trunk][wanted for 2.14.5][wanted for 2.16.2]
(Assignee)

Comment 10

15 years ago
On HEAD:
 Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.212; previous revision: 1.211
done 

On BUGZILLA-2_14_1-BRANCH:
Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.99.2.2; previous revision: 1.99.2.1
done                                                      

On BUGZILLA-2_16-BRANCH :
Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.149.2.12; previous revision: 1.149.2.11
done                

                                     
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
OS: other → All
Hardware: PC → All
Whiteboard: [wanted for trunk][wanted for 2.14.5][wanted for 2.16.2] → [fixed on trunk][fixed in 2.14.5][fixed in 2.16.2]
(Assignee)

Comment 11

15 years ago
Created attachment 109929 [details] [diff] [review]
Further patch for TIP.. only remove the ~
(Assignee)

Comment 12

15 years ago
Created attachment 109930 [details] [diff] [review]
Further patch for 2.14
(Assignee)

Comment 13

15 years ago
OOPS forgot about localconfig.js must be accessable
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 14

15 years ago
Created attachment 109931 [details] [diff] [review]
Further patch for 2.14
Note that some editors use "~localconfig" or "#localconfig" or other schemes.

Surely it's the responsibility of an admin to configure their editor not to
leave backups of sensitive files lying around? :-)

Are we going to have to rename localconfig.js to make this work, or can we add
an exception for it?

Gerv
Ok... so are (yet again) holding the release for this?

There's a reason I put "released this weekend" in the status report... I wanted
to give us the flexibility for stuff like this. :-)
(Assignee)

Comment 17

15 years ago
Created attachment 109937 [details] [diff] [review]
Replacement patch for TIP

This patch requires backing out the old patch first

This uses .*localconfig.*
but then countermands that in a subsequent block with an allow from all
Attachment #109929 - Attachment is obsolete: true
Attachment #109930 - Attachment is obsolete: true
Attachment #109931 - Attachment is obsolete: true
Ok... just talked to joel on IRC, and I guess the checked in patch isn't good
enough; when this bug is closed again, with the proper fix checked in on all the
branches, I'll go ahead and retag all the releases, which I have to do anyway
because we forgot relnotes for 2.14.5 and 2.16.2.
Comment on attachment 109937 [details] [diff] [review]
Replacement patch for TIP

Tested on landfill w/ 2.17.2; looks good.

r=preed
Attachment #109937 - Flags: review+
(Assignee)

Comment 20

15 years ago
Created attachment 109938 [details] [diff] [review]
Replacement patch for 2_16

Same for 2.16 
Back out prior patch first
Attachment #109897 - Attachment is obsolete: true
Attachment #109899 - Attachment is obsolete: true
Attachment #109900 - Attachment is obsolete: true
(Assignee)

Comment 21

15 years ago
Created attachment 109939 [details] [diff] [review]
Replacement patch for 2_14

Same story, back out prior patch first

Comment 22

15 years ago
Comment on attachment 109938 [details] [diff] [review]
Replacement patch for 2_16

TIP version works ok, TIP/2.16/2.14 look ok and apply ok.

r=burnus

(Hopefully not to many have already run ./checksetup from tip, otherwise
localconfig.js is unavailable. Maybe
+    if ($oldaccess =~ s/\|localconfig\|/\|.*localconfig.*\|/) {
should be changed to
+    if ($oldaccess =~ s/\|localconfig(\.\|)?\|/\|.*localconfig.*\|/) {
At least I have run already the wrong update.)
Attachment #109938 - Flags: review+

Updated

15 years ago
Attachment #109939 - Flags: review+
Comment on attachment 109937 [details] [diff] [review]
Replacement patch for TIP

Note that this patch will add the FilesMatch for allowing the .js and .rdf at
the end of the .htaccess file...  if a site is blocking IPs because of robots
or DOSes (like mothra), this will put the new check after those, thus allowing
the banned IPs to still access those files (because an "allow from all" was the
last match).  Probably doesn't matter a whole lot.  People who are maintaining
that type of thing can fix it themselves. :-)
(Assignee)

Comment 24

15 years ago
Fixed in all 3 branches (again)
Status: REOPENED → RESOLVED
Last Resolved: 15 years ago15 years ago
Resolution: --- → FIXED
(Assignee)

Comment 25

15 years ago
Public notice complete. Removing security flag.
Group: webtools-security

Comment 26

14 years ago
lack of responce
lack of response to what?  it's already fixed.
QA Contact: matty_is_a_geek → default-qa
Comment hidden (spam)
You need to log in before you can comment on or make changes to this bug.