Page MenuHomePhabricator

Alert group Cookie(s) without Secure flag set
Closed, ResolvedPublic

Description

Web Server
Alert group Cookie(s) without Secure flag set
Severity Low
Description
This cookie does not have the Secure flag set. When a cookie is set with the Secure flag, it instructs
the browser that the cookie can only be accessed over secure SSL channels. This is an important
security protection for session cookies.
Recommendations If possible, you should set the Secure flag for this cookie.
Alert variants
Details
stopMobileRedirect=true; expires=Thu, 24-Oct-2019 08:47:45 GMT; path=/; domain=xxx;
HttpOnly
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-pServer_Side=false;vector-nav-p-Programming=false;vector-nav-ptb=false;xxx_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

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?

sbassett triaged this task as Medium priority.Nov 12 2019, 4:35 PM
sbassett added a project: MobileFrontend.
sbassett moved this task from Backlog / Other to Other WMF team on the acl*security board.
sbassett added a subscriber: Jdlrobson.

The stopMobileRedirect cookie allows editors to opt out of the mobile site on their phone - by requesting they always get desktop. There might be a more modern way to do this with headers now though...

The stopMobileRedirect cookie allows editors to opt out of the mobile site on their phone - by requesting they always get desktop. There might be a more modern way to do this with headers now though...

Ok, that's fair. I was more curious as to why the cookie's secure flag is being explicitly set to false within the code. I'd assume because there were issues with it not working properly for non-TLS sites, though I have no knowledge of the history here. If there really isn't a good reason for it being set to false here, we should probably change it with a gerrit patch.

I'm not sure about the secure flag. Git blame suggests this has been around since July 2014 and was added by @MaxSem - maybe he remembers?

Probably has something to do with us not being HTTPS only back then.

Probably has something to do with us not being HTTPS only back then.

Sounds likely. If there aren't any objections, I'd like to push a patch through gerrit setting the secure key to true (or just removing it) and then make this task public. I'm less worried about this one than T238075 since we should be tls-only on all production projects and this code has been in the wild for many years.

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".Nov 25 2019, 10:34 PM
sbassett updated the task description. (Show Details)

Change 552906 had a related patch set uploaded (by SBassett; owner: SBassett):
[mediawiki/extensions/MobileFrontend@master] SECURITY: Set stopMobileRedirect cookie within MobileFrontent to true

https://gerrit.wikimedia.org/r/552906

Change 552906 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] SECURITY: Set stopMobileRedirect cookie within MobileFrontent to true

https://gerrit.wikimedia.org/r/552906

Jdlrobson edited projects, added MobileFrontend (Tracking); removed MobileFrontend.
Jdlrobson moved this task from Tracking to team:other on the MobileFrontend board.
Jdlrobson edited projects, added MobileFrontend; removed MobileFrontend (Tracking).

Can this be resolved?

sbassett claimed this task.

Can this be resolved?

Yes.