1517993 - An unexpected error occurred with browser.cookies.set() with domain and url key for ip addresses

Status: RESOLVED FIXED

(WebExtensions :: Compatibility, defect, P5)

Description

[Kenny D]

Attachment: browser.cookies.set_bug.PNG

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:65.0) Gecko/20100101 Firefox/65.0

Steps to reproduce:

1. Have an extension with cookies and all urls permission
2. Run this in either the extension code or in the console

await browser.cookies.set({
  "domain": "192.168.1.1",
  "httpOnly": true,
  "name": "",
  "path": "/",
  "sameSite": "lax",
  "secure": true,
  "storeId": "firefox-default",
  "url": "https://192.168.1.1/",
  "value": ""
})


Actual results:

Error: An unexpected error occurred


Expected results:

A cookie should have been set. The above works in Chrome though.

This call without the domain key works in Firefox.

await browser.cookies.set({
  "httpOnly": true,
  "name": "",
  "path": "/",
  "sameSite": "lax",
  "secure": true,
  "storeId": "firefox-default",
  "url": "https://192.168.1.1/",
  "value": ""
})

Another thing to note is that this only happens for IP domains.

Comment 1

[Rob Wu [:robwu]]

This call without the domain key works in Firefox.

This is how the cookies API is supposed to be used. When the "domain" key is passed to the details parameter of browser.cookies.set, the extension API will treat it as a domain cookie (opposed to a host-only cookie). IP addresses are not domain names (they cannot have subdomains), so only host-only cookies are supported for these addresses.

Our implementation unconditionally prepends a period (".") [1], which causes the cookie to be interpreted as a domain cookie. To force the cookie to be treated as a host-only cookie, check if the host is an IP address and avoid prepending a period (e.g. by returning early).

[1] https://searchfox.org/mozilla-central/rev/c21d6620d384dfb13ede6054015da05a6353b899/toolkit/components/extensions/parent/ext-cookies.js#130-132

Comment 2

[Rob Wu [:robwu]]

Note: Do not forget to account for IPv6 addresses.

Comment 3

[Rob Wu [:robwu]]

For those who want to fix this bug, I described the issue and how to fix it in more detail at https://phabricator.services.mozilla.com/D21828#752489

Note: The browser.cookies.set implementation currently also has an issue with brackets: https://searchfox.org/mozilla-central/rev/2738efcf98d746c8810819e7c0634b2c57212a8d/toolkit/components/extensions/parent/ext-cookies.js#86-100,130-132

• Line 86-88 : If domain is not passed to browser.cookies.set (i.e. cookie.host is empty), the parsed URI's host is used, which does not have brackets - OK.
• Line 100 : The cookie.host (should be with brackets) is compared with uri.host (which doesn't have brackets). This is wrong. At that point, cookie.host should be normalized with dropBracketsIfIPv6.
• Line 130-132 : This is the cause of https://bugzilla.mozilla.org/show_bug.cgi?id=1517993

Together with comment 1, the path to a patch should be quite doable.

If you are new here and interested in picking up this bug, see https://wiki.mozilla.org/WebExtensions/Contribution_Onramp

Comment 4

[Myeongjun Go]

Could I ask you this issue?
I want to check contents I understand.
Because IP address are not domain names.
If host(i.e. cookie.host) is IP address(IPv6 or IPv4), the host(i.e. cookie.host) be not added a leading ".".

If I understood right, I would try to solve this issue.

Comment 5

[Rob Wu [:robwu]]

I believe that you have understood the issue, and have assigned the bug to you.

Comment 6

[Myeongjun Go]

Attachment: Bug 1517993 - An unexpected error occured with browser.cookies.set() with domain and url key for ip address. r=robwu

Comment 7

[Myeongjun Go]

Thanks for checking my code:) I appreciate it.
I revised again.
If It's wrong or need correction, please tell me again.

Comment 8

[Rob Wu [:robwu]]

(Already replied in Phabricator)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2f69f765ef51
An unexpected error occured with browser.cookies.set() with domain and url key for ip address. r=robwu,rpl,mixedpuppy

Comment 10

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/2f69f765ef51