691973 - Expired Cookies Not Removed by Cookie Manager

Status: NEW

(Core :: Networking: Cookies, defect, P5)

Description

[David E. Ross]

Attachment: screenshot showing bug

Mozilla/5.0 (Windows NT 5.1; rv:7.0) Gecko/20110923 SeaMonkey/2.4

I visited several wiki.mozilla.org pages.  Later, I noticed that a cookie from that domain was still present although it had expired about 50 minutes earlier.  I terminated SeaMonkey and then relaunched it.  The cookie was still present.  

Expired cookies should be deleted or at least marked so as (1) not be displayed by the Cookie Manager and (2) -- much more important -- not sent to the indicated server if the page is again requested.  

The attachment shows the current date and time (4 Oct 2011 4:18:52 PM) in the Date and Time Properties window over the Cookie Manager display from the Data Manager window.  At the bottom of the Data Manager window, the cookie expiration is given (same date, 3:29:04 PM).

Comment 1

[Bill Lee]

The bug shows platform as x86 Windows XP, but it exists on my x86 Fedora system as well.

Comment 2

[David E. Ross]

I just now reviewed the cookies in one of my profiles.  There were several still present with expiration dates and times a few hours earlier, a few days earlier, and several months earlier.  

Lacking any workaround (other than reviewing each cookie and manually deleting the expired cookies), I have changed the priority to Major.  In light of comment #1, I have changed the OS to All.

Comment 3

[Chris Peterson [:cpeterson]]

Bug 576347 proposes to delete expired cookies once a day when Firefox is idle.

Comment 4

[David E. Ross]

Windows 7 (x64)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 SeaMonkey/2.22

Bug #576347 is marked "Mac OS X", appears to be an RFE (despite being marked "Normal"), is not assigned, and appears not to have any action for implementation.  This bug, however, has been seen with Windows XP and Windows 7 and is a true discrepancy.  

One of my profiles currently has 76 persistent cookies with 10 of them expired.  At least one of the expired 3 months ago.  The most recent expiration was about a week ago.  With all the effort being put into adding security features to Mozilla-based applications, I would think this bug would have significant priority for a correction.

Comment 5

[marvinhk]

just discovered this issue in my ff box.
i consider #576347 a supplementary solutions for performance and/or profile maintenance.
here are my ideas.

baseline (site-based check):
whenever a site is visited, ALL cookies under the site should be checked for expiry.
expired cookies may have impact (passing wrong security or session or other info) to the application.

at start or end of session (session-based check):
clean up expired cookies as if they are (expired) session cookies

implement #576347 (periodic check):
periodic clean up for use case in which ff is rarely restart (mainly laptop users that only hibernate)
frequency should be configurable via about:config

Comment 6

[David E. Ross]

Re comment #5:

Yes, this would be an acceptable solution.

Comment 7

[Ian Nartowicz]

The reasons for re-opening this bug no longer apply.  The duplicate is marked for all platforms although it still doesn't look like getting fixed or even worked on any time soon.

Comment 5 is not a likely solution.  Cookies are already identified as expired when a site is visited so that they are not passed to the web server, but in most cases they are already in the database.  In the middle of loading a web page is not the best time to be deleting entries from the database, which is presumably why we have this bug.

Similarly, startup and shutdown are time-critical and we can't be adding IO at those times.  Session cookies are slightly different because they aren't in the database.  The only action needed to purge them is to not ever write them into the database.

Bug 576347 suggests periodic cleanup of the database during idle.  My addon currently does this and it is a very quick process with no impact that I can see.  My addon doesn't do database vacuuming, I thought that was a concept that was dropped in favour of pre-allocation?

Comment 8

[David E. Ross]

Re last paragraph of comment #7:  

What add-on?

Comment 9

[marvinhk]

(In reply to Ian Nartowicz from comment #7)
> Comment 5 is not a likely solution.  Cookies are already identified as
> expired when a site is visited so that they are not passed to the web
> server, but in most cases they are already in the database.  In the middle
> of loading a web page is not the best time to be deleting entries from the
> database, which is presumably why we have this bug.
>
> Similarly, startup and shutdown are time-critical and we can't be adding IO
> at those times.  Session cookies are slightly different because they aren't
> in the database.  The only action needed to purge them is to not ever write
> them into the database.
>
So what you mean is there is never a good time (except the idle timeout) to clean the ever growing cookie database unless we only allow session cookies. Even if these cookies are never sent, there is still risk to privacy / information leaks via cookie history which cannot be ignored.

I'm a bit confused here. On one hand the clean up can be done very quick (with an addon), but on the other hand it will have significant impact to page load/startup/shutdown. It seems to me the primary concern lies with the database operations. I am naive enough to think this bug means something like "delete * from cookie where expiry_time <= now". Other database maintenance tasks (allocation, vacuumuing, defrag, etc) are not point of interest as far as I'm concerned.

The proposal on site-based check is intended to limit the extend of individual clean-up (and off-load the clean-up at shutdown). I agree that it may not be a good idea if it is not time-effective. But at the minimum, I think shutdown clean up is important, esp for this privacy related container. Good applications perform clean up at exit and performance may not be a good reason for the trade off. I think there are much more concern on the cookie privacy compared to process shutdown time.

See also cookie tagging. One of the goal is to faciliate logout clean-up.
<https://wiki.mozilla.org/Privacy/Features/Cookie_tagging>
Perhaps we can have the shutdown cleanup in place first, and ride on this feature for performance improvement in longer run.

PS: currently, I'm performing some manual clean-up with Cookie Time
<https://addons.mozilla.org/en-US/firefox/addon/cookie-time/>

Comment 10

[Ian Nartowicz]

Half a second (usually less, but occasionally more) once a day when someone is not using their computer is fast and negligible.  The same amount of work during startup or shutdown is not acceptable.

Comment 11

[David E. Ross]

I have three browser profiles.  One of them is needed for banking and investment transactions because the Web sites involved require preferences to be set differently from how I normally browse.  Among the different settings is to allow ALL cookies.  

Earlier today, I examined the cookies in that profile and found several that were not relevant to the purpose of the profile and should be deleted.  To do that, I had to wade through several expired cookies that also had to be deleted.  The time I spent at this task was significantly greater than any performance hit that might occur if expired cookies were purged at startup or shutdown.  

When I spent several minutes manually removing expired cookies, quibbling about a few seconds of delay in a startup or shutdown is no justification for not fixing this bug.

Comment 12

[Ian Nartowicz]

Get a grip.

Comment 13

[Artem Polivanchuk]

Firefox 30 stable.
Expired cookies are present in the list, while the "remove after expiration" option is activated.
It's a serious problem and should be fixed ASAP.

Comment 14

[Ian Nartowicz]

Well it's certainly *a* problem.  The only real question is when and how those cookies should be purged from the database.  Input would be more useful than just re-stating the bug.  Perhaps a patch is what is really need to focus minds.

Comment 15

[chAlx]

I suggest at least to mark it as a security hole and as a regression. Then provide a workaroud for end users (some SQLite tool or something).

Bug 576347 is not a dupe, it suggests one way to resolve this bug. So it should be added to Dependency list.

Comment 16

[David E. Ross]

It seems to me that any performance hit caused by purging expired cookies and vacuuming the database on start-up -- with a database that is NOT pre-allocated to a fixed (and sometimes excessively large) size -- would be mitigated by the reduced database when querying for required cookies and when inserting new cookies.  

I have already observed delays in completing the launch of SeaMonkey for two of my profiles that have many bookmarks in places.sqlite and almost immediate completion for a profile that has very few bookmarks.  Thus, the size of a database does create a performance hit.  Where that size involves obsolete entries, reducing the size by eliminating those entries surely must improve performance.

Comment 18

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 19

[flitterio]

Can anyone comment whether an expired (but not deleted) cookie is visible to JavaScript (even if it's JS from the same origin as the cookie)?  If so, would that raise the priority of this issue?

Comment 20

[Martin F.]

(In reply to flitterio from comment #19)
> Can anyone comment whether an expired (but not deleted) cookie is visible to
> JavaScript (even if it's JS from the same origin as the cookie)?

In my tests expired cookies were not accessible via document.cookie (in the developer tools console).

Comment 21

[Graham Perrin]

Might this bug explain the misbehaviour that I found today with Firefox 72?

From https://old.reddit.com/r/firefox/comments/ejy2kv/wayback_machine_pages_webarchiveorg_urls_work/fd4s5hz/?context=3:

… Where Firefox failed, today, Cookie Quick Manager succeeded. …

It's remarkable that the extension found anything for the sites, so soon after Firefox had supposedly removed the data. …

Comment 22

[Marcela (Please NI request to Brindusa Tot)]

Hi David!
I'm trying to reproduce old bugs to see if we can resolve any. Could you please tell us if this is still happening with the latest version of firefox?
Thanks!

Comment 23

[David E. Ross]

First of all, I am a SeaMonkey user. While I now have Firefox, I marked its cookies.sqlite as "read only". I only use Firefox for those very few Web sites that refuse to render with SeaMonkey.

On top of all that, I am still with SeaMonkey 2.49.5 because later versions broke extensions that I very frequently use.

Thus, I cannot be any help with this.

Comment 24

[Qwerty]

Expired cookies are still not purged in 92 and Nightly.

Comment 25

[Marcela (Please NI request to Brindusa Tot)]

Hi Jens!
It seems this bug is still active. Is there any ongoing investigation about this?
Thanks!

Comment 26

[Jens Stutte [:jstutte]]

(In reply to Qwerty from comment #24)

Expired cookies are still not purged in 92 and Nightly.

Jan, can you confirm this?

Comment 27

[Dragana Damjanovic [:dragana]]

Necko needs to investigate this.

Comment 28

[takatomatsuki]

This is still present in Firefox 102.0.1 - Cookies past their expire date are still present in the sqlite and displayed in Firefox. Expired cookies are a privacy/security concern as they may contain login details or other private data. I don't think they are accessible by websites, but there is nothing to prevent applications or viruses from reading the sqlite file. During browser startup or shutdown is when I would expect these cookies to be deleted. Short term solution could be to have a "Clear Expired Cookies" button on the "Manage Cookies & Site Data" panel.

Comment 29

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 16 votes.
:dragana, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 30

[Dragana Damjanovic [:dragana]]

Ed, can you take a look?

Comment 31

[edgul]

I have confirmed this is still an issue in 106 nightly. Expired cookies are NOT sent to the server, but are visible in console.

Though it's still unclear what the solution is, I see at least 3 proposals:

1. Cleanup on startup or shutdown (potential performance issues)
2. Cleanup during idle
3. Manual button trigger

Since this issue is so old it may be worth summarizing methods for cookie cleanup that currently exist

1. Manual cookie deletion
2. Settings > Clear Data
3. Cookie purging on addCookie described here

Dan, do you have any thoughts on this?

Comment 32

[edgul]

De-prioritizing. Though if we get a patch proposal we would take the fix.

Comment 33

[Daniel Veditz [:dveditz]]

Dan, do you have any thoughts on this?

Scratch the "Manual button trigger" option: in practice we might as well do nothing than spend time coding a solution hardly anyone will think about using.

Startup would be bad, shutdown might be OK, but probably we just need to do a periodic sweep. We could also tweak the purging on addCookie to be more aggressive (don't stop at 3000, clear them all out, or at least until it's ~10% or so under the limit)