1645915 - Visiting youtube.com. (trailing dot) on Windows breaks IndexedDB for all sites and extensions

Status: RESOLVED DUPLICATE of bug 1645943

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

Description

[Iain Nicol]

Attachment: firefox localstorage log.txt

This is just a denial of service—but once activated, it stops IndexedDB in all extensions and websites, and persist across restarts. This was discovered by accident, after reading a reminder on Reddit about FQDNs.

Steps to reproduce:

1. Visit https://www.youtube.com./ The trailing dot is required. So is the https, but only because of the way they redirect.
2. Wait a few seconds, for the page to load and cache to persist to disk, etc.
   3a. IndexedDB will not work on any other website. For example, https://codepen.io/GlennAxworthy/pen/NppRXa will show no rows (instead of two), and the Add button will not work.
   3b. IndexedDB will not work in any extension. For example, log into Bitwarden (if you can), and your vault will show as empty (even if it should not be!). Try a manual sync, and the vault will remain apparently empty, and the date of last sync will still say that it has never synced.

Presumably it is the initial indexedDB.open() call which fails in the JavaScript, not a later one.

The denial of service aspect, of course, would come from tricking a user into visiting a link like in step 1.

** Underlying cause: **

Visiting the website creates the following folder:

C:\Users\Username\AppData\Roaming\Mozilla\Firefox\Profiles\profilename\storage\default\https+++www.youtube.com.

(The trailing dot above is literal, not a full stop.)

However, Win32 is kinda well known (see e.g. https://superuser.com/questions/230385/dots-at-the-end-of-file-name/1533170#1533170) to not allow trailing dots. This can be directly observed by the fact Windows Explorer will struggle to browse, delete, or rename the guilty the folder. Similarly, PowerShell struggles as well; the Linux layer WSL can handle it, however.

Creating the folder with the trailing dot probably required Firefox to use the special path prefix "\?", which bypasses some Win32 restrictions.

Anyway, in Firefox, the existence of this folder causes NS_ERROR_FILE_ACCESS_DENIED, which causes the IndexedDB methods to fail. See the logs.

** More information: **

This error persists if you restart Firefox. Nor does clearing the cache etc (Ctrl+Shift+Delete) necessarily fix it. We are not certain, but it might be that clearing the cache etc fixes it before you restart Firefox, but not after you restart Firefox.

Separately, for comparison, I see that in bug #1624586 (and its dupes), there is also some discussion of folders with trailing dots. To be clear, I think this issue we are raising here is new compared to that other bug. In the very least, this issue is 100% reproducible, whereas the cause of the other bug is unexplained. Moreover, I think that here the folder is 'ACCESS_DENIED' which breaks IndexedDB, whereas in the other bug the folders were 'unexpected' which messes up LocalStorage.

** Supporting log: **

The JavaScript console shows the following error (e.g. from the Bitwarden extension):

UnknownError: The operation failed for reasons unrelated to the database itself and not covered by any other error code. ExtensionStorageIDB.jsm:831
normalizeStorageError resource://gre/modules/ExtensionStorageIDB.jsm:831
method chrome://extensions/content/child/ext-storage.js:273
AsyncFunctionThrow self-hosted:697

A Firefox debug log is also attached, from visiting the above codepen page. But, in essence, that JS error above corresponds to this native errors:

[Parent 12072, QuotaManager IO] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520015 (NS_ERROR_FILE_ACCESS_DENIED): file /builds/worker/checkouts/gecko/storage/mozStorageService.cpp, line 638
[...]
[Parent 12072, IPDL Background] WARNING: Converting non-IndexedDB error code (0x80520015) to NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR: file /builds/worker/checkouts/gecko/dom/indexedDB/ActorsParent.cpp, line 561

Versions:

Operating system: Windows 10 (2004). We suspect this reproduces on many other versions of Windows.
Firefox: v77.0.1 (x64); Nightly v78.0b (x64)

Comment 1

[Johann Hofmann [:johannh]]

Andrew, can you take a look at this?

Thanks!

Comment 2

[Jens Stutte [:jstutte]]

Maybe a reproducible trigger for a QM initialization failure?

Comment 3

[Jan Varga [:janv]]

This should have been fixed in bug 1536796.
Tom, can you take a look ?

Comment 4

[Tom Tung [:tt, :ttung]]

Visiting the website creates the following folder:
C:\Users\Username\AppData\Roaming\Mozilla\Firefox\Profiles\profilename\storage\default\https+++www.youtube.com.

This proves that patches in bug 1536796 work because it makes it possible to create such files. (before that we cannot create these files and their storage would be taken account as "http+++youtube.com"'s storage [no dot])

One thing that is not clear to me is why this causes initialization failures. QuotaManager and its client (IndexedDB) should be able to access these files. I will figure this out.

Interestingly, the frames below look similar as the report in https://bugzilla.mozilla.org/show_bug.cgi?id=1536796#c128,

 [stack trace unavailable]
[Parent 12072, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file /builds/worker/checkouts/gecko/dom/cache/FileUtils.cpp, line 810
[Parent 12072, QuotaManager IO] WARNING: 'NS_FAILED(mozilla::dom::cache::LockedDirectoryPaddingGet( dir, &paddingSize))', file /builds/worker/checkouts/gecko/dom/cache/QuotaClient.cpp, line 375
[Parent 12072, QuotaManager IO] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520015 (NS_ERROR_FILE_ACCESS_DENIED): file /builds/worker/checkouts/gecko/storage/mozStorageService.cpp, line 638
[Parent 12072, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file /builds/worker/checkouts/gecko/dom/cache/DBAction.cpp, line 249
[Parent 12072, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file /builds/worker/checkouts/gecko/dom/cache/QuotaClient.cpp, line 136
[Parent 12072, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file /builds/worker/checkouts/gecko/dom/cache/QuotaClient.cpp, line 378
[Parent 12072, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file /builds/worker/checkouts/gecko/dom/quota/ActorsParent.cpp, line 5311

So, these frames show that there was a QuotaManager initialization failure.

Comment 5

[Tom Tung [:tt, :ttung]]

I can reproduce this issue with "https://www.youtube.com./"

Comment 6

[Iain Nicol]

This proves that patches in bug 1536796 work because it makes it possible to create such files

Ah, very interesting!

Also, not a technical point, but it is impressive to file a bug at bedtime, and for some initial triage to have happened before waking up. Especially because I realise an accidental DoS is way less dangerous than an information leak or code execution etc.

Comment 7

[Tom Tung [:tt, :ttung]]

(In reply to Tom Tung [:tt, :ttung] from comment #5)

I can reproduce this issue with "https://www.youtube.com./"

While I was reproducing with that website, I hit an assertion https://searchfox.org/mozilla-release/rev/d39f65d8ae122c3983a710b886554157f4229df9/dom/quota/ActorsParent.cpp#4711.

The steps for me are:

1. Open Firefox
2. Go to the website
3. Hit the assertion
4. Open Firefox again
   5, Go to any website that triggers storage initialization
5. The storage Initialization fail

In 2-3, QM creates https+++www.youtube.com. and its contents successfully.
In 6, the file path has \\?\ prefix and the file path matches my expectation (\\?\{$profiles}\storage\default\https+++youtube.com.\cache\.padding).

The issues I saw here are:

• hit the assertion
• the write access in 2 succeeded, but the read access in 6 failed

I will start from figuring out why we hit the assertion.

Comment 8

[Jan Varga [:janv]]

Regarding the assertion, maybe SQLite removes the prefix, so we don't get it when GetQuotaObject is called from TelemetryVFS.cpp ?

Comment 9

[Tom Tung [:tt, :ttung]]

(In reply to Tom Tung [:tt, :ttung] from comment #7)

• the write access in 2 succeeded, but the read access in 6 failed

In my case, there is no any file under https+++youtube.com.\cache\, so it doesn't look like it's related to the prefix \\?\ things. I suspect it's because we don't handle an edge case like this in initialization. (no database, no padding file)

(In reply to Jan Varga [:janv] from comment #8)

Regarding the assertion, maybe SQLite removes the prefix, so we don't get it when GetQuotaObject is called from TelemetryVFS.cpp ?

It does look like that. I will verify it.

Comment 10

[Tom Tung [:tt, :ttung]]

(In reply to Tom Tung [:tt, :ttung] from comment #9)

(In reply to Jan Varga [:janv] from comment #8)

Regarding the assertion, maybe SQLite removes the prefix, so we don't get it when GetQuotaObject is called from TelemetryVFS.cpp ?

It does look like that. I will verify it.

path: \\?\C:\XXXX\https+++youtube.com\cache\cache.sqlite vs directoryPath: \\?\C:\XXXX\https+++youtube.com.\cache\

The ending period for the origin directory name in path was removed somehow.

Comment 11

[Tom Tung [:tt, :ttung]]

(In reply to Tom Tung [:tt, :ttung] from comment #10)

path: \\?\C:\XXXX\https+++youtube.com\cache\cache.sqlite vs directoryPath: \\?\C:\XXXX\https+++youtube.com.\cache\

The ending period for the origin directory name in path was removed somehow.

It might be because SQLite doesn't use the prefix, so the file path for the SQLite database in DOM Cache was changed from https+++youtube.com.\cache\cache.sqlite to https+++youtube.com\cache\cache.sqlite without returning any error, and then the transferred file path is used as the aPath argument to GetQuotaObject.

In the opt-build, this causes QM to return a wrong QuotaObject (the one without ending with a period) and thus cause the database access to fail so that we don't have a database file in this case.

Comment 12

[Jens Stutte [:jstutte]]

youtube.com. with trailing dot is not even a valid domain name. Is it possible, that at some (late) point in the chain from the URL bar to our code some sanitization on the domain name takes place? Such that we end up in some places with a path without trailing dot (for creating the DB), in others with (for reading it subsequently)?

Comment 13

[Iain Nicol]

Eh, it depends what you mean by not valid. Absolutely agreed that, in practice, people don’t use them in web browsers and URLs.

However, the trailing dot makes it the FQDN (Full Qualified Domain Name). The trailing dot represents the root domain, i.e the domain that tells you where to find the nameserver for .com.. The FQDN an old concept, and is in RFCs. See e.g. https://en.wikipedia.org/wiki/Fully_qualified_domain_name

IIRC, Firefox used to complain about domain mismatch when visiting an HTTPS FQDN. However, that appears not to happen any more—I wonder if that is change is deliberate or not.

Comment 14

[Iain Nicol]

To be clear, I think it might be reasonable to normalise the domain name, to get rid of the dot. I am just saying the trailing dot is awkward, as opposed to outright wrong.

Comment 15

[Jens Stutte [:jstutte]]

(In reply to Iain Nicol from comment #13)

Eh, it depends what you mean by not valid. Absolutely agreed that, in practice, people don’t use them in web browsers and URLs.

However, the trailing dot makes it the FQDN (Full Qualified Domain Name). The trailing dot represents the root domain, i.e the domain that tells you where to find the nameserver for .com.. The FQDN an old concept, and is in RFCs. See e.g. https://en.wikipedia.org/wiki/Fully_qualified_domain_name

IIRC, Firefox used to complain about domain mismatch when visiting an HTTPS FQDN. However, that appears not to happen any more—I wonder if that is change is deliberate or not.

Thanks for the reminder. I should not use the first tool I found on the web but better read the spec.

Comment 16

[Daniel Veditz [:dveditz]]

Anyone who knows about this bug could navigate victims to a site that does this (including the youTube one) and kill indexedDB for that user. It would be hard for average users to figure out how to get themselves out of this situation -- this has a pretty high impact even if I can't give it a high "severity" from the security POV (it is limited and recoverable, technically).

Comment 17

[Tom Tung [:tt, :ttung]]

This is actually a dup of bug 1645943 and the fix is now in 79

Comment 18

[Tom Tung [:tt, :ttung]]

I'm going to close this since bug 1645943 is fixed.

Comment 19

[Daniel Veditz [:dveditz]]

Even though this bug was marked a duplicate, it was actually reported before the bug that got the fix and thus qualifies for a bug bounty.