Closed Bug 667087 Opened 8 years ago Closed 8 years ago

Cookies cannot be stored with single character hostnames

Categories

(Core :: Networking: Cookies, defect)

1.9.2 Branch
All
Linux
defect
Not set

Tracking

()

VERIFIED FIXED
Tracking Status
firefox5 --- unaffected
blocking1.9.2 --- .20+
status1.9.2 --- .20-fixed

People

(Reporter: chrisccoulson, Assigned: Ehsan)

References

Details

(Keywords: regression, reproducible, verified1.9.2)

Attachments

(2 files)

I'm reporting this from https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/801778

Since Firefox 3.6.18, cookies cannot be stored with single character hostnames. This is reproducible by installing the extension developer addon and doing this in the JS shell:

cm = Cc["@mozilla.org/cookiemanager;1"].getService(Ci.nsICookieManager2);
expiry = (Date.now() + 1000) * 1000;
cm.countCookiesFromHost("a"); /* Should be 0 */
cm.add("a", "/", "foo", "bar", false, false, true, expiry);
cm.countCookiesFromHost("a"); /* Is still 0, but should now be 1 */

This wasn't a problem in 3.6.17. It doesn't seem to be a problem on current nightlies or aurora either (I didn't test 5.0)

I see there's a couple of suspect commits between 3.6.17 and 3.6.18, but I'm still waiting for my mozilla-1.9.2 tree to build before I can actually verify that:

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/bb935ffe5ff1
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/bb728fdcd717
It's definitely this one which breaks it: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/bb935ffe5ff1

When running the first iteration of the loop in nsCookieService::CountCookiesFromHostInt, (nextDot <= end && *(nextDot + 1) == '\0') always evaluates to true, so it just bails out
Blocks: 650522
blocking1.9.2: --- → ?
Hardware: x86_64 → All
Here is a patch which fixes this, with test-case. I'm not sure if it's the best way to fix this without understanding what http://hg.mozilla.org/releases/mozilla-1.9.2/rev/bb728fdcd717 is for.
Attachment #542125 - Flags: review?(dwitte)
Attachment #542128 - Flags: review?(dwitte)
apparently not a problem on trunk, where we took a different patch for bug 616264
Assignee: nobody → ehsan
Blocks: CVE-2011-2362
No longer blocks: 650522
blocking1.9.2: ? → .19+
Blocks: 650522
No longer blocks: CVE-2011-2362
Comment on attachment 542125 [details] [diff] [review]
Fix bug 667087.patch

Looks great to me. Nice fix! r=dwitte
Attachment #542125 - Flags: review?(dwitte) → review+
Comment on attachment 542128 [details] [diff] [review]
Test for mozilla-central

r=dwitte
Attachment #542128 - Flags: review?(dwitte) → review+
Attachment #542125 - Flags: approval1.9.2.19?
The ubuntu folks want this badly.  Can we get the approval on this one soon, please?
I landed the m-c test on inbound.  Please do not mark the bug as FIXED when you merge to central.
Whiteboard: DO NOT MARK AS FIXED WHEN MERGING TO CENTRAL
Comment on attachment 542125 [details] [diff] [review]
Fix bug 667087.patch

Approved for 1.9.2.19
Attachment #542125 - Flags: approval1.9.2.19? → approval1.9.2.19+
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/2b1eb891d7fe
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: DO NOT MARK AS FIXED WHEN MERGING TO CENTRAL
Verified fixed using 1.9.2 testcase for 1.9.2.20.
Keywords: verified1.9.2
(In reply to comment #12)
> Verified fixed using 1.9.2 testcase for 1.9.2.20.

Changing the resolution to verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.