1818657 - "WebDriver:GetCookies" returns secure cookies on a non-secure document

Status: NEW

(Remote Protocol :: Marionette, defect, P2)

Description

[Marco Squarcina]

Attachment: poc.py

This is a follow up of bug#1783536.

According to my tests, WebDriver returns Secure cookies when the current browsing context’s active document is non-secure. The issue can be easily reproduced with the selenium script attached to this report.

Running the script, the Secure cookie test is printed twice on Firefox, whereas on Chrome the Secure cookie is returned only on https://example.com. The output of the script is the following:

[S] Cookies on https://example.com: [{'name': 'test', 'value': 'secure', 'path': '/', 'domain': 'example.com', 'secure': True, 'httpOnly': False, 'sameSite': 'Strict'}]
[I] Cookies on http://example.com: [{'name': 'test', 'value': 'secure', 'path': '/', 'domain': 'example.com', 'secure': True, 'httpOnly': False, 'sameSite': 'Strict'}]

I explicitly added the SameSite attribute to level any differences between the two browsers. According to rfc6265bis, http://example.com should be considered a different site wrt https://example.com. So FF WebDriver is showing a cookie scoped for a different site.

I also had a look at the WebDriver spec and found out that all associated cookies to a document must meet the requirements specified by the algorithm in rfc6265, Sec 5.4 (step 1) . Despite referencing the old standard from 2011, the first step mentions that Secure cookies must be discarded on non-secure documents. This bug is somehow related to https://github.com/mozilla/geckodriver/issues/1840 and bug#1690739.

Although this issue looks like a fun CTF challenge, I suppose there may be real-world security implications affecting pipelines using FF WebDriver.

Comment 1

[:Gijs (he/him)]

Isn't this a dupe of bug 1690739 ? Or am I missing something?

Comment 2

[Marco Squarcina]

(In reply to :Gijs (he/him) from comment #1)

Isn't this a dupe of bug 1690739 ? Or am I missing something?

bug 1690739 is about setting Secure cookies from a non-secure browsing context, while this one concerns reading Secure cookies from a non-secure browsing context. Consider also that, historically, Secure cookies were allowed to be set over HTTP, while it has never been the case that Secure cookies were readable from a non-secure context. See the second paragraph of rfc6265, Sec 4.1.2.5 (now removed from rfc6265bis).

I see these as complementary issues (that's why I linked 1690739 in the report), but not exactly the same bug.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

We do not think that this is a security related bug given that you can use WebDriver to navigate anyway to the HTTPS site and call the "Get All Cookies" command as well. In such a case the test would know about the secure cookie as well.

Also for our WebDriver BiDi implementation we would like to have an API similar to CDP which allows to pass a list of domains/hosts and the command will return all the relevant cookies.

As such it's a bug that needs to be fixed but not security related.

The problem is actually in the implementation of cookie.iter() where we only pass-in the host but not the schema. As such we do not filter out secure cookies.

I'm going to put this into our P2 list so that we can find time for it in our next milestone.

Comment 4

[Marco Squarcina]

Makes sense, I prudently marked it as a security bug since it can be seen as a confidentiality violation of secure cookies. But threat-modeling wise, this shouldn't really be problematic. Thanks for taking care of the bug!

Comment 5

[:Gijs (he/him)]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #3)

As such it's a bug that needs to be fixed but not security related.

OK, removing the relevant group then.

The problem is actually in the implementation of cookie.iter() where we only pass-in the host but not the schema. As such we do not filter out secure cookies.

I'm unfamiliar with webdriver, but wouldn't the expectation from users of the API be that get_cookies uses the context in which it is invoked to produce cookies? In particular, would the expectation be that SameSite cookies being returned depended on whether they were sent with the document at the time? I'd have similar questions around containers/userContextId, and private browsing.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to :Gijs (he/him) from comment #5)

The problem is actually in the implementation of cookie.iter() where we only pass-in the host but not the schema. As such we do not filter out secure cookies.

I'm unfamiliar with webdriver, but wouldn't the expectation from users of the API be that get_cookies uses the context in which it is invoked to produce cookies? In particular, would the expectation be that SameSite cookies being returned depended on whether they were sent with the document at the time? I'd have similar questions around containers/userContextId, and private browsing.

I would forward this question to James. Maybe he already has some ideas in how to define that command for WebDriver BiDi (which then WebDriver classic might benefit from as well) so that we can take care of all these extra parameters.

Comment 7

[James Graham [:jgraham]]

I'm unfamiliar with webdriver, but wouldn't the expectation from users of the API be that get_cookies uses the context in which it is invoked to produce cookies? In particular, would the expectation be that SameSite cookies being returned depended on whether they were sent with the document at the time? I'd have similar questions around containers/userContextId, and private browsing.

Well the spec just says to serialize:

each cookie in all associated cookies of the current browsing context’s active document:

Where "associated cookies" is defined as:

the enumerated set of cookies that meet the requirements set out in the first step of the algorithm in [RFC6265] to compute cookie-string for an ‘HTTP API’, from the cookie store of the given document’s address. The returned cookies must include HttpOnly cookies.

So it does implicitly assume that it only depends on the document's address, which isn't quite true given cookie store partitioning by request destination. However for private browsing and containers I think it's totally reasonable to interpret the spec as "send the cookies that would actually be sent in this context". Also note that WebDriver doesn't provide an (easy) way to create a browsing context in a container, or mix private and non-private browsing modes, so the question doesn't often arise.

For BiDi we will indeed need to do better, since the plan is to allow getting cookies associated with an arbitary origin, not just for the active document in a given context. So we will need to be more explicit in allowing you to specify which cookies exactly you want to get.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Note that there is also https://github.com/w3c/webdriver/issues/1571 open to clarify the exact behavior when a secure cookie should be set for HTTP. Right now the Marionette implementation would still set such a cookie on a non-secure connection.

Maybe it would make sense work on both APIs when getting this fixed so that proper tests can be created.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

We might be able to fix that with the work on bug 1854580 (network.getCookies) and bug 1854582 (network.setCookies) for WebDriver BiDi. As such lets remove from the m9 list for now.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

We are currently not using a shared module for cookies at the moment given that the implementations between BiDi and Marionette differ quite a lot. So both of the now removed bugs will not help here.