Closed Bug 1522829 Opened 6 years ago Closed 5 years ago

Cookie's attribute samesite=strict is not enforced properly for links opened in new tabs/windows in Fenix

Categories

(Fenix :: General, defect, P2)

Unspecified
Android
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: labor.omnia.vincit18, Unassigned)

References

()

Details

(Keywords: csectype-disclosure, sec-low)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:64.0) Gecko/20100101 Firefox/64.0

Steps to reproduce:

Hello,

When a domain set any of it's cookie as samesite=strict. It expects the browser to not send this cookie, if request is originated from cross domain (irrespective of whether the request is GET, POST etc.).
But, in firefox (for iOS and Android) when user opens the link by "right-click -> open a link in new tab/window", then it does sends the samesite protected cookies in the request.

Steps to reproduce:

  1. Visit https://humblehunter.io/temp/samesite.php (this will set a cookie with samesite=strict attribute).
  2. Host the file having below content in some other domain (say attacker.com).

<html>
<body>
<a href="https://humblehunter.io">test</a>
</body>
</html>

  1. Open the above anchor tag link directly in same tab (without right click). Then, cookie won't go in the request.

  2. However, if you right-click on the anchor link and open it in new tab/window.
    You'll see the GET request to https://humblehunter.io contains the cookie which was supposed to be protected by samesite attribute.

Actual results:

In case of open in new tab, GET request to target domain contains the cookie which was supposed to be protected by samesite attribute.

Expected results:

In case of 4th point above, the cookie should not have left the browser.
Tested the same in FireFox (MacOSX version) as well, and it was working as expected.

Impact:
If a website does a state change on server side based on some input taken from GET request. And, it relies on samesite=strict cookie attribute for providing CSRF protection. Then, the same website is still vulnerable to CSRF.

User interaction:
User needs to explicitly open the link in new tab to bypass the samesite=strict cookie protection.

Version:
Firefox version: 14.0 (iOS 12.1.3), 64.0.2 (Android 6.0.1, April 1, 2017 security patch).
Chrome/system webview android version: 71.0.3578.99 (Android 6.0.1, April 1, 2017 security patch).

Note: I've written "right click" above, you can do the mobile equivalent of it.

(In reply to Abhinash Jain from comment #0)

But, in firefox (for iOS and Android) when user opens the link by "right-click -> open a link in new tab/window", then it does sends the samesite protected cookies in the request.

Note that the iOS version relies on the builtin webkit, so I'm not sure how much we can do here - does mobile safari even implement samesite=strict generally, and/or provide APIs to tell it to treat a load as non-same-site?

Tested the same in FireFox (MacOSX version) as well, and it was working as expected.

OK, so for the android version, I expect our 'open in a new tab' code in Android is not passing the right triggering principal / origin information. Mark/Jonathan, do you have cycles to investigate?

Flags: needinfo?(mgoodwin)
Flags: needinfo?(jkt)

The current versions of all major browsers (including iOS and Android defaults) now support the same-site cookie attribute.

So given comment 3 the iOS issue is common to webviews you should report that to Apple. We'll investigate the Android issue here.

In some ways opening a link in a new window is considered the same as copying the URL and then pasting it into a new tab you opened manually. Not clear whether we should treat that based on the context or as if it's the shortcut for that manual process (which would of course send all the same-site cookies).

Group: firefox-core-security → mobile-core-security

Abhinash, is this samesite=strict bug also reproducible in the new "Firefox Preview" app?

https://play.google.com/store/apps/details?id=org.mozilla.fenix

Also, the https://humblehunter.io/temp/samesite.php test site in comment 0 no longer exists.

(In reply to Chris Peterson [:cpeterson] from comment #6)

Abhinash, is this samesite=strict bug also reproducible in the new "Firefox Preview" app?

https://play.google.com/store/apps/details?id=org.mozilla.fenix

Also, the https://humblehunter.io/temp/samesite.php test site in comment 0 no longer exists.

It's not reproducible in Preview 1.1.0 (Build #12042111).
I observed that the samesite cookie does not even leave the browser when domain is accessed directly by typing through address bar (kindly confirm).
This could also be the reason that it's not sent when accessed through "open in new tab".

The humblehunter.io don't exist anymore, it was anyway just setting the cookie something like:

<?php
$cookie_name = "somename";
$cookie_value = "somevalue";
setcookie($cookie_name, $cookie_value, ["lifetime" => time() + (86400 * 30), "path" => "/", "domain" => "", "secure" => true, "httponly" => true, "samesite" => 'Strict']);
?>

(In reply to Abhinash Jain from comment #7)

(In reply to Chris Peterson [:cpeterson] from comment #6)

Abhinash, is this samesite=strict bug also reproducible in the new "Firefox Preview" app?

https://play.google.com/store/apps/details?id=org.mozilla.fenix

Also, the https://humblehunter.io/temp/samesite.php test site in comment 0 no longer exists.

It's not reproducible in Preview 1.1.0 (Build #12042111).
I observed that the samesite cookie does not even leave the browser when domain is accessed directly by typing through address bar (kindly confirm).
This could also be the reason that it's not sent when accessed through "open in new tab".

That sounds problematic from a correctness perspective. :snorp, given you looked at principal/csp stuff in GV recently, can you check what's going on with samesite=strict cookies on GV/Fenix? If they're completely broken, that likely means site breakage... Might want to be a separate bug, I guess?

Flags: needinfo?(snorp)

It would be better to use Reference Browser to test instead of Firefox Preview. It runs a more recent version of Gecko.

AFAIK we don't do anything that would cause a problem here. When you enter an address through the location bar, csp isn't in play and the triggering principal is a null one.

Flags: needinfo?(snorp)

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #10)

AFAIK we don't do anything that would cause a problem here. When you enter an address through the location bar, csp isn't in play and the triggering principal is a null one.

I'd expect the TP to be system principal as it's a user action, and I wouldn't be surprised if the use of a null principal explained not sending samesite=strict cookies with that initial request. That seems like a compat issue, because if the user requests example.com directly from the location bar, samesite=strict cookies for example.com should be sent, not omitted. I've filed public bug 1573860 for this.

See Also: → 1573860
Summary: Cookie's attribute samesite=strict is not enforced properly → Cookie's attribute samesite=strict is not enforced properly for links opened in new tabs/windows on fennec/iOS

In GV bug 1573860, we found that GV was defaulting to a null principal for user-initiated page loads from the URL bar, but desktop defaults to the system principal for these URL bar page loads.

Moving this bug to the Fenix::Security component because snorp says this is a Fenix bug, not a GV bug. Fenix needs to pass the referring tab's GeckoSession when opening the new tab.

We won't fix Fennec Android (because it's in ESR maintenance mode) and we can't fix Fennec iOS (because it uses WebKit).

Component: General → Security: Android
Product: Firefox for Android → Fenix
Summary: Cookie's attribute samesite=strict is not enforced properly for links opened in new tabs/windows on fennec/iOS → Cookie's attribute samesite=strict is not enforced properly for links opened in new tabs/windows in Fenix
Version: Firefox 64 → unspecified

Fenix needs to pass the referring tab's GeckoSession when opening the new tab.

Christian, I remember this is something we fixed in A-C. Is that correct? Can we close this report?

Flags: needinfo?(sarentz) → needinfo?(csadilek)

Christian, I remember this is something we fixed in A-C. Is that correct? Can we close this report?

The part we fixed was to provide the right flags when calling loadUri. For this one (passing along the parent / referring session), there's still some work left to do in A-C and Fenix: https://github.com/mozilla-mobile/android-components/issues/4749

Flags: needinfo?(csadilek)

This has been fixed now in A-C and QA process is tracked in https://github.com/mozilla-mobile/fenix/issues/5019. Resolving this issue, as discussed with :kbrosnan.

Status: UNCONFIRMED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Group: mobile-core-security → core-security-release
Flags: needinfo?(jonathan)
Flags: needinfo?(mgoodwin)

Removing employee no longer with company from CC list of private bugs.

Group: core-security-release
Component: Security: Android → General
OS: Unspecified → Android
You need to log in before you can comment on or make changes to this bug.