Closed Bug 368502 Opened 18 years ago Closed 16 years ago

Bugzilla_logincookie should not be accessible via javascript

Categories

(Bugzilla :: Bugzilla-General, defect)

2.23.3
defect
Not set
major

Tracking

()

RESOLVED FIXED
Bugzilla 3.2

People

(Reporter: bugzilla-mozilla, Assigned: reed)

Details

(Keywords: selenium, Whiteboard: [wanted-bmo])

Attachments

(1 file, 3 obsolete files)

Bugzilla should prevent the Bugzilla_logincookie from being accessible from javascript. This can be done by using the httponly flag. This is currently only supported by IE6SP1+. This requires CGI 3.21 or newer. My intention is only to set that http flag when the CGI version is new enough.

See also:
bug 178993
http://search.cpan.org/src/LDS/CGI.pm-3.25/Changes
http://msdn.microsoft.com/workshop/author/dhtml/httponly_cookies.asp
Note Bug 178993 tracks httponly support for Firefox.
Bugzilla 3.2 is now frozen. Only enhancements blocking 3.2 or specifically approved for 3.2 may be checked in to the 3.2 branch. If you would like to nominate your enhancement for Bugzilla 3.2, set the "blocking3.2" flag to "?". Then, either the target milestone will be changed back, or the blocking3.2 flag will be granted, if we will accept this enhancement for Bugzilla 3.2.

This particular bug has not been touched in over eight months, and thus is being retargeted to "---" instead of "Bugzilla 4.0". If you believe this is a mistake, feel free to retarget it to Bugzilla 4.0.
Target Milestone: Bugzilla 3.2 → ---
Can we get this in for Bugzilla 3.2?
Severity: enhancement → major
Flags: blocking3.2?
Attached patch patch - v1 (obsolete) — Splinter Review
This adds -httponly to the CGI send_cookie() code.
Assignee: general → reed
Status: NEW → ASSIGNED
Attachment #335120 - Flags: review?(mkanat)
Whiteboard: [wanted-bmo]
Comment on attachment 335120 [details] [diff] [review]
patch - v1

A few questions:

1) What about future JSON calls that need to use XHR, do those still work?

2) I'm pretty sure that -httponly should have a value, like 1.

3) CGI.pm 3.21 does not come with Perl 5.8.8, which is annoying, but since we have install-module I suppose it's OK.
Attachment #335120 - Flags: review?(mkanat) → review-
Not a blocker.
Flags: blocking3.2? → blocking3.2-
(In reply to comment #5)
> 1) What about future JSON calls that need to use XHR, do those still work?

Hopefully bz can answer this.

> 2) I'm pretty sure that -httponly should have a value, like 1.

Yeah, duh. Oops. :(

> 3) CGI.pm 3.21 does not come with Perl 5.8.8, which is annoying, but since we
> have install-module I suppose it's OK.

Most people have newer CGI.pm versions anyway, thanks to distributions shipping them.
Attached patch patch - v1.1 (obsolete) — Splinter Review
Dumb mistake. :(
Attachment #335120 - Attachment is obsolete: true
Attachment #335122 - Flags: review?(mkanat)
XHR will send the cookie, I believe.  It just won't expose it to the script.
(In reply to comment #1)
> Note Bug 178993 tracks httponly support for Firefox.

If a web browser doesn't support httponly, what happens?
(In reply to comment #10)
> (In reply to comment #1)
> > Note Bug 178993 tracks httponly support for Firefox.
> 
> If a web browser doesn't support httponly, what happens?

Nothing. The browser will (hopefully, if it parses the cookie spec properly) ignore the extra "HttpOnly;" part in the cookie, so it's not protected by this security feature.
Attached patch patch - v1.2 (obsolete) — Splinter Review
mkanat wanted me to move the line addition above the -expires.
Attachment #335122 - Attachment is obsolete: true
Attachment #335127 - Flags: review?(mkanat)
Attachment #335122 - Flags: review?(mkanat)
Comment on attachment 335127 [details] [diff] [review]
patch - v1.2

Okay, great, but also update the relnotes.html.tmpl to note that the version for CGI is new. (Very simple, you'll see.)

Also add a comment that we require CGI.pm 3.21 to support httponly (in Install::Requirements, there.)
Attachment #335127 - Flags: review?(mkanat) → review-
Attached patch patch - v1.3Splinter Review
Review comments addressed.
Attachment #335127 - Attachment is obsolete: true
Attachment #335128 - Flags: review?(mkanat)
Comment on attachment 335128 [details] [diff] [review]
patch - v1.3

Yeah, looks fine to me. :-)
Attachment #335128 - Flags: review?(mkanat) → review+
Flags: approval?
Flags: approval3.2?
Flags: approval?
Flags: approval3.2?
Flags: approval3.2+
Flags: approval+
Target Milestone: --- → Bugzilla 3.2
tip:

Checking in Bugzilla/Auth/Persist/Cookie.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Auth/Persist/Cookie.pm,v  <--  Cookie.pm
new revision: 1.6; previous revision: 1.5
done
Checking in Bugzilla/Install/Requirements.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/Requirements.pm,v  <--  Requirements.pm
new revision: 1.50; previous revision: 1.49
done
Checking in template/en/default/pages/release-notes.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/release-notes.html.tmpl,v  <--  release-notes.html.tmpl
new revision: 1.23; previous revision: 1.22
done

BUGZILLA-3_2-BRANCH:

Checking in Bugzilla/Auth/Persist/Cookie.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Auth/Persist/Cookie.pm,v  <--  Cookie.pm
new revision: 1.5.4.1; previous revision: 1.5
done
Checking in Bugzilla/Install/Requirements.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/Requirements.pm,v  <--  Requirements.pm
new revision: 1.47.2.3; previous revision: 1.47.2.2
done
Checking in template/en/default/pages/release-notes.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/release-notes.html.tmpl,v  <--  release-notes.html.tmpl
new revision: 1.17.2.7; previous revision: 1.17.2.6
done
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
(In reply to comment #5)
> 1) What about future JSON calls that need to use XHR, do those still work?

Confirming bz, cookies are automagically handled by XHR.

(In reply to comment #11)
> > If a web browser doesn't support httponly, what happens?
> 
> The browser will (hopefully, if it parses the cookie spec properly)
> ignore the extra "HttpOnly;" part in the cookie, so it's not protected by this
> security feature.

No "hopefully" about it, httponly is in wide use and browsers that don't
understand it treat them as regular cookies, just as older versions of Firefox
did. For examples of httponly in use see the many social networking sites like
Facebook and LiveJournal.
Flags: testcase?
Hmm, would this break Greasemonkey scripts that access this cookie?  Or is it exempt because of being part of an extension?  I have more than one Bugzilla account, and I use a GreaseMonkey script that keeps track of which user I'm logged in as and changes the border color on my page to remind me.  At the time it was written, Bugzilla didn't embed anything in the page that was easy to scrape to figure it out, so the login cookie was the easiest way to get it.  I'll have to push to get that info embedded in the HTML somehow if it'll break my script :)
Actually, Bugzilla_login is your userid, it has nothing to do with your session (other than an extra identifier to make sure the session token you're using actually belongs to you).  There's not anything given up by allowing that one to be read, is there?  Bugzilla_logincookie is the only one that's dangerous to expose.
Keywords: relnote
Added to release notes for 3.2 in bug 463244.
Keywords: relnote
Flags: testcase? → testcase+
Keywords: selenium
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: