1517145 - about:newtab indexeddb data grows to excessive size because IDB doesn't do a FIle/Blob integrity check for permanent storage

Status: NEW

(Core :: Storage: IndexedDB, defect, P3)

Description

[Dave Townsend [:mossop]]

I was investigating some issues my wife is having with her Firefox profile and happened to notice that it was quite large. Turns out the indexed db store for about:newtab (storage/default/about+newtab) is taking up 1.1GB of space. This seems too large. What on earth are we putting in there?

Comment 1

[Nan Jiang [:nanj]]

:andreio, could you investigate this, please?

Comment 2

[Andrei Oprea [:andreio]]

@mossop is it possible to share that data? I was not able to reproduce the issue. Activity Stream only stores some boolean preferences in IndexedDB, no sensitive data is present.
Also it might be interesting to list the timestamps of those files (were they all created at the same time or has it been gradually growing).

Comment 3

[Andrei Oprea [:andreio]]

Have you come across a similar issue? Some follow up information

most of the files are in <profile>/storage/default/about+newtab/idb/3312185054sbndi_pspte.files. Each file is only 4-5MB but there are 220 of them
indexeddb inspector shows basically no data for Activity Stream

That last part makes me think it might not be a AS specific bug, this code has been in production for several releases without any similar reports

Comment 4

[Ed Lee :Mardak]

Maybe a sanity check of visiting https://firefox-storage-test.glitch.me/ and seeing if everything is green?

Comment 5

[Dave Townsend [:mossop]]

(In reply to Andrei Oprea [:andreio] from comment #2)

@mossop is it possible to share that data? I was not able to reproduce the issue. Activity Stream only stores some boolean preferences in IndexedDB, no sensitive data is present.

Here: https://drive.google.com/file/d/1ZGAyOO3OixlDRpiT05mmil_-x7_KUruS/view?usp=sharing

Also it might be interesting to list the timestamps of those files (were they all created at the same time or has it been gradually growing).

Unfortunately I no longer have this, looks like the timestamps show when I copied the files from my wife's machine and the profile has been since deleted from her machine.

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

The contents of the database indicate two things are going on:

• The key "snippets" has a 4 megabyte string inside that seems to be serialized HTML that contains a bunch of SVG images that are stored inline as base-64 data URL's. This gets stored as a file on disk because, for efficiency, IndexedDB stores large structured clones as externally stored Blobs.
• There may be a very serious bug in IndexedDB related to external storage of structured clonesbecause there should only be one of those giant 4 megabyte files on disk.

:mossop, because the "snippets" key appears to be the persisted state of the activity stream UI at various points during use, it is in fact likely to contain private information and I would suggest deleting/revoking access to the archive. I am marking this bug as a security bug in the interim to avoid further potential exposure of the contents. I don't seem to have the ability to mark just the single comment as security sensitive. Note that I haven't actually seen any user-private data from the blobs and don't intend to, so this is not any reaction to any private information, I'm just acting out of caution for private information.

I do not believe we need to perform any further analysis of the file. At the raw SQLite level, the contents of the "file" table is (from .dump):
INSERT INTO file VALUES(403,1);

And the sole row referencing a file is:
INSERT INTO object_data VALUES(1,X'30746f6a7171667574',NULL,'.403',4294967296);
(Noting that the opaque string is a fancy encoding of "snippets").

Comment 7

[Dave Townsend [:mossop]]

I have revoked access to the file (it was already only visible to mozilla staff).

Comment 8

[Jan Varga [:janv]]

It would be helpful to see directory listing of <profile>/storage/default/about+newtab/idb/3312185054sbndi_pspte.files

Comment 9

[Dave Townsend [:mossop]]

Attachment: file listing

Comment 10

[Tom Tung [:tt, :ttung]]

Probably not related to this bug, but note that the broken profile (missing fileInfo) I got in bug 1515069 is also for about+home, the database is used for snippets, and the size of the problematic files in the xxx.files directory is 4.8 + 4.8 MB (there are two files).

Comment 11

[Nan Jiang [:nanj]]

I believe that :janv has come up with a workaround for this issue, could you put more details here, :janv?

Comment 12

[Jan Varga [:janv]]

Yeah, we need to fix this.

Comment 13

[David Durst [:ddurst]]

It's unclear to me how or when this bug will be handled. Is this something DOM is doing as part of 1541370 (and thus we should assign and prioritize as part of that) or something that AS will handle with janv's guidance?

We should coordinate this -- whatever it is we do or find out -- with bug 1522188.

Comment 14

[Jan Varga [:janv]]

I'm afraid this is a low level IndexedDB bug and must be handled by someone from workers & storage team.

For now, you can try to set "dom.indexedDB.dataThreshold" pref to "-1".

Comment 15

[Andrew Overholt [:overholt]]

Is this situation unique to mossop's case? Do we have telemetry on the size of the snippets db? How urgent of an IDB change is this?

Comment 16

[Nan Jiang [:nanj]]

(In reply to Andrew Overholt [:overholt] from comment #15)

Is this situation unique to mossop's case? Do we have telemetry on the size of the snippets db? How urgent of an IDB change is this?

We don't have telemetry coverage on the db size in Activity Stream, can't tell if it's common or not.

The other telemetry shows that the "INDEXEDDB_OPEN_FAILED" and "TRANSACTION_FAILED" are the most frequent IDB failures seen in Activity Stream. Per asuth, the former is related to IDB quota manager, and the team has been working on the fix to it. Not sure if the transaction failure has to do with this issue.

Comment 17

[Andrew Sutherland [:asuth] (he/him)]

Moving this to IDB because I think most of the actionable work here is in IDB/ended up in IndexedDB.

Restating my understanding of the underlying problems:

• It's likely that File/Blob references were retained to IDB-file-backed File/Blobs whose lifetimes would extend up to and/or beyond profile-before-change-qm. When freed, it would be too late to delete the contents from the database.
• IDB normally self-heals this problem during origin initialization, but this is only done for storage/default, not storage/permanent.

In general, we've helped improve a lot of IDB shutdown/cleanup life-cycle issues, but there's still the potential problem of system-principaled pages holding File/Blob references beyond the point that content should have been torn down. It's not immediately clear to me if we've improved things so that all of these references are just severed at shutdown, allowing us to clean up the orphaned blobs during shutdown or not. But it's clear that we need to be performing origin initialization for these databases at least some of the time in order to clean up orphaned blobs.

Unfortunately this then runs into latency problem concerns. Maybe idle maintenance should be fixing this?

Comment 18

[Jan Varga [:janv]]

FileInfo/FileManager which was/is likely the cause of the reported problems have been heavily cleaned up in bug 1617170. Most importantly, there's now a gtest which verifies FileInfo/FileManager expected behavior. So it's possible that the reported issue no longer exists.