Bugzilla_logincookie should not be accessible via javascript

RESOLVED FIXED in Bugzilla 3.2

Status

()

Bugzilla
Bugzilla-General
--
major
RESOLVED FIXED
11 years ago
5 years ago

People

(Reporter: Olav Vitters, Assigned: reed)

Tracking

({selenium})

2.23.3
Bugzilla 3.2
selenium
Bug Flags:
approval +
approval3.2 +
blocking3.2 -
testcase +

Details

(Whiteboard: [wanted-bmo])

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

11 years ago
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.

Comment 2

10 years ago
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 → ---
(Assignee)

Comment 3

10 years ago
Can we get this in for Bugzilla 3.2?
Severity: enhancement → major
Flags: blocking3.2?
(Assignee)

Comment 4

10 years ago
Created attachment 335120 [details] [diff] [review]
patch - v1

This adds -httponly to the CGI send_cookie() code.
Assignee: general → reed
Status: NEW → ASSIGNED
Attachment #335120 - Flags: review?(mkanat)
(Assignee)

Updated

10 years ago
Whiteboard: [wanted-bmo]

Comment 5

10 years ago
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-

Comment 6

10 years ago
Not a blocker.
Flags: blocking3.2? → blocking3.2-
(Assignee)

Comment 7

10 years ago
(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.
(Assignee)

Comment 8

10 years ago
Created attachment 335122 [details] [diff] [review]
patch - v1.1

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.

Comment 10

10 years ago
(In reply to comment #1)
> Note Bug 178993 tracks httponly support for Firefox.

If a web browser doesn't support httponly, what happens?
(Assignee)

Comment 11

10 years ago
(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.
(Assignee)

Comment 12

10 years ago
Created attachment 335127 [details] [diff] [review]
patch - v1.2

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 13

10 years ago
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-
(Assignee)

Comment 14

10 years ago
Created attachment 335128 [details] [diff] [review]
patch - v1.3

Review comments addressed.
Attachment #335127 - Attachment is obsolete: true
Attachment #335128 - Flags: review?(mkanat)

Comment 15

10 years ago
Comment on attachment 335128 [details] [diff] [review]
patch - v1.3

Yeah, looks fine to me. :-)
Attachment #335128 - Flags: review?(mkanat) → review+
(Assignee)

Updated

10 years ago
Flags: approval?
Flags: approval3.2?

Updated

10 years ago
Flags: approval?
Flags: approval3.2?
Flags: approval3.2+
Flags: approval+
Target Milestone: --- → Bugzilla 3.2
(Assignee)

Comment 16

10 years ago
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
Last Resolved: 10 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.

Updated

10 years ago
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.

Updated

10 years ago
Keywords: relnote

Comment 20

9 years ago
Added to release notes for 3.2 in bug 463244.
Keywords: relnote

Updated

6 years ago
Flags: testcase? → testcase+

Updated

5 years ago
Keywords: selenium
You need to log in before you can comment on or make changes to this bug.