Web Server
Alert group Cookie(s) without HttpOnly flag set
Severity Low
Description
This cookie does not have the HTTPOnly flag set. When a cookie is set with the HTTPOnly flag, it
instructs the browser that the cookie can only be accessed by the server and not by client-side scripts.
This is an important security protection for session cookies.
Recommendations If possible, you should set the HTTPOnly flag for this cookie.
Alert variants
Details mf_useformat=true; expires=Sat, 23-Nov-2019 09:47:45 GMT; path=/; domain=; Secure
GET /w/index.php?
go=%D8%A8%D8%B1%D9%88&mobileaction=toggle_view_mobile&search=the&title=%D9%88%DB%8C%DA%98%D9%87:
%D8%AC%D8%B3%D8%AA%D8%AC%D9%88 HTTP/1.1
Cookie: vector-nav-p-HTML_.D9.88_CSS=true;vector-navp-.D8.AC.D8.A7.D9.88.D8.A7_.D8.A7.D8.B3.DA.A9.D8.B1.DB.8C.D9.BE.D8.AA=false;vector-nav-p-
Server_Side=false;vector-nav-p-Programming=false;vector-nav-ptb=false;wikicod_coddb_wikicod__session=rqe73jqgdkt7g5f4svo838m7lv53h58t;stopMobileRedirect=true
Authorization: Basic YW5vbnltb3VzOmFub255bW91cw==
Accept: */*
Accept-Encoding: gzip,deflate
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.21 (KHTML, like Gecko)
Chrome/41.0.2228.0 Safari/537.21
Connection: Keep-alive
Description
Details
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Reedy | T256334 Release MediaWiki 1.31.9/1.34.3/1.35.0 | |||
| Resolved | sbassett | T256342 Write and send supplementary release announcement for extensions and skins with security patches (1.31.9/1.34.3/1.35.0) | |||
| Resolved | sbassett | T238075 Alert group Cookie(s) without HttpOnly flag set |
Event Timeline
I'm guessing this is a false positive as it's being intentionally set to false here. @Jdlrobson - is this still necessary for certain MobileFrontend functionality?
Yep the mf_useformat cookie is used by 3rd parties or wikis which do not have a mobile domain setup.
Similar to T238076#5667248, is there a good reason not to be setting the HttpOnly flag to true? If there really isn't a good reason for it being set to false here, we should probably change it with a gerrit patch.
Hmm, not sure we can change this one, since it looks like at least a few tests might break. Though I'm not certain how relevant, current or necessary any of that code is.
Provided this isn't impacting the Varnish logic, @sbassett those tests in MobileFrontend and Minerva look like they can be revised, so please go ahead and write the patch. My team will be happy to help out with fixing the tests. Note: not sure what mediawiki-moderation is.
@Jdlrobson - do we know if any of the tests would currently be used by jenkins-bot to -1 anything in CI? i.e. are they used in quibble or anywhere else? If not, then I'd be fine sending up another patch in gerrit sometime early next week, just before the train properly returns.
Proposed patch: P12038. Again, I'm only seeing this referenced within a few Selenium tests where it might break. Otherwise I'll plan to deploy as a security patch during this Monday's window.
@sbassett I don't think, but I'm not too familiar with the Selenium stack @zeljkofilipin do you know if this would create problems for the Selenium tests?
@Jdlrobson as far as I can see at the codesearch, in the context of Selenium tests, mf_useformat is only used in MobileFrontend and MinervaNeue to set cookies related to using desktop or mobile mode. I'm not really familiar with tests in those repositories.
I can try running the patch targeting a local mediawiki-docker instance, but I don't know how to get P12038.
@zeljkofilipin - I've subbed you on the paste, though let me add a proper file here as I've found that copy-pasting patches from text out of Phab can be problematic in getting them to properly apply:
I was still going to apply it to production this afternoon during the security window as a security patch. The (possible) issues with tests breaking would only come into play for backports to master, etc.
Ok. Since the patch has been successfully deployed to production (for now), we can make this task public and start on the backports if you'd like. That way everything could be easily run and tested via gerrit/jenkins. I'm also going to track this issue here: T256342.
Change 616889 had a related patch set uploaded (by SBassett; owner: SBassett):
[mediawiki/extensions/MobileFrontend@master] mf_useformat cookie should be HttpOnly
Change 616889 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] mf_useformat cookie should be HttpOnly
Change 617137 had a related patch set uploaded (by SBassett; owner: SBassett):
[mediawiki/extensions/MobileFrontend@REL1_35] mf_useformat cookie should be HttpOnly
Change 617138 had a related patch set uploaded (by SBassett; owner: SBassett):
[mediawiki/extensions/MobileFrontend@REL1_34] mf_useformat cookie should be HttpOnly
Change 617139 had a related patch set uploaded (by SBassett; owner: SBassett):
[mediawiki/extensions/MobileFrontend@REL1_31] mf_useformat cookie should be HttpOnly
The last thing to do here, once the backports are all merged, is to request a CVE. Though this is a very borderline case in my opinion since it was more an (intentionally) insecure configuration and not an actual security bug within the code.
Change 617138 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@REL1_34] SECURITY: mf_useformat cookie should be HttpOnly
Change 617137 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@REL1_35] SECURITY: mf_useformat cookie should be HttpOnly
Change 617139 merged by SBassett:
[mediawiki/extensions/MobileFrontend@REL1_31] SECURITY: mf_useformat cookie should be HttpOnly
Ok, all of the backports appear to be merged. Thinking about T238075#6345083 some more, I don't think this merits a CVE, though I still plan to include it within the announcement for T256342.
I have dropped the patch from /srv/patches/1.36.0-wmf.4/extensions/MobileFrontend since it is now included in the branch.