Storage for extensions cookies is weird, non-performant, and possibly has a security problem |
|||||||
Issue descriptionAs a result of some (mostly) wild goose chases debugging for http://codereview.chromium.org/2898953008 I've realized that cookies created with non-canonical schemes (anything other than file:, filesystem:, http:, https:, mailto:, ftp:, ws:, wss:, http-so:,https-so:; see DoCanonicalize in url/url_util.cc) from URLs without an explicit "domain" field won't have a Domain() field (everything after the // is put in the path), and as a result will all be stored using the same, null, key in the cookie monster. This has performance implications for storage and lookup of chrome-extensions: cookies (though I don't think that's particularly important), and *might* have security implications. I did a nihinihi manual test by inspecting the backing pages of a couple of extensions I have running (no cookies set in document.cookie), and set cookies in one and did not see it in others. So the obvious pathway for leakage between extensions doesn't seem to work; presumably there's some path based filter on it. But I don't have the depth of understanding of the "extensions cookies" path to be sure that there isn't a problem here. I'm filing this bug to track digging down to the floor on this one, but will not be pursuing it with high priority. If someone wants to take it from me, feel free. I'll remove the security permission when I've either decided there's no chance of leakage of cookie values between extensions or gotten a security review :-}.
,
Feb 16 2018
Removing myself as owner, adding other possibly interested cookie parties.
,
Apr 18 2018
,
Apr 19 2018
,
Apr 19 2018
mmenke@, can you add an explanation of why this is now considered fixed?
,
Apr 19 2018
We don't allow cookies to be set with any non-canonical scheme, other than the extensions scheme, but that's just for extensions that call document.cookie, and uses its own cookie store. Hrm....I may have misunderstood the description.
,
Apr 20 2018
First of all, the original report is mistaken about the key used in the database: $ sqlite3 ~/.config/chromium/Default/Extension\ Cookies -cmd "SELECT host_key FROM cookies;" aknmeieabphheepdkibhjbpoagohaido Now the key used by SQLitePersistentCookieStore and CookieMonster was inconsistent, with one being an empty string, indeed, but that got fixed in https://chromium-review.googlesource.com/c/chromium/src/+/940141, standardizing on GetKey which handles extension stuff distinctly: https://cs.chromium.org/chromium/src/net/cookies/cookie_monster_unittest.cc?rcl=d223f8bf342614d9831f5c8534ddfa438d373dcb&l=2002
,
Jul 27
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by mmenke@chromium.org
, Jun 7 2017