820658 - Private Browsing leaks history in SQLite's Write-Ahead Log for tabs.db

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Private Browsing leaks history in the tabs.db-wal file.

This file is the SQLite "Write-Ahead Log" and is described at http://www.sqlite.org/draft/wal.html

The leak happens when you have private browsing tabs open and browse to different sites.

While you browse the tabs.db is updated. Since it replaces the record for tab in a transaction, the value is also written to the Write-Ahead Log.

The WAL seems to be growing in size and is cleaned up periodically. But with enough luck you can find a good number of URLs of previously visited sites from private browsing sessions.

I was able to do this with the following command:

  strings tabs.db-wal | strings | grep http

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

We should look into how we are saving data into tabs.db and make sure we apply the same PB rules as for history.

I also think this borders on "takes a lot of effort to see the leaked data" and I might be OK to ignore it, if we can't easily fix the problem.

Comment 2

[(no longer active)]

We should definitely fix this.  Most of the data that we prevent to get saved to disk fits the "takes a lot of effort to see the leaked data" category...

Comment 3

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

My interpretation of the -wal file was wrong. I thought it was a temporary file but instead it contains uncommitted records that WILL be part of the actual tabs.db when SQLite runs a checkpoint.

For example I currently have five tabs open in Firefox:

about:home
Hacker News (PRIVATE)
Add-ons
Copy Profile :: Add-ons for Android
New RA Flood Attack (PRIVATE)

When I use sqlite to look at the tabs.db file I see this:

sqlite> select _id,url from tabs;
40|about:home
41|http://news.ycombinator.com/
42|about:addons
43|https://addons.mozilla.org/en-US/android/addon/copy-profile/?src=search
44|about:reader?url=http%3A%2F%2Fsamsclass.info%2Fipv6%2Fproj%2FRA_flood2.htm%231&readingList=0&tabId=10

So this means that these private tabs are not just in a temporary file. They are actually persisted in the real tabs.db.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Yep. That tabs.db is used to allow Sync to replicate tabs from mobile to desktop. We should not store private tabs in the tabs.db.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

I only found one place where we add tabs to the tabs.db and that is here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/Tabs.java#411

That code eventually leads here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/TabsAccessor.java#186

This patch adds a tab.isPrivate() check here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/TabsAccessor.java#195

That will skip private tabs when saving to the DB

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1fb70af2d687

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1fb70af2d687