Bug 216962

Summary:

IndexedDB Index Corruption after upgrade from iOS 13 to iOS 14

Product:

WebKit

Reporter:

Darryl Pogue <dvpdiner2>

Component:

New Bugs

Assignee:

Sihui Liu <sihui_liu>

Status:

RESOLVED FIXED

Severity:

Critical

CC:

alecflett, beidson, darin, ews-watchlist, jsbell, niklasmerz, sihui_liu, simon.fraser, tim.brust, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

Safari 14

Hardware:

iPhone / iPad

OS:

Other

Attachments:

Description	Flags
Testcase file	none
Patch	none
Patch	none
Patch	none

Description Darryl Pogue 2020-09-24 22:57:02 PDT

r255318 introduced a change to how IDB Indexes are stored in the SQLite backing store. It moves from per-ObjectStore index IDs to per-Database index IDs, and attempts to perform a migration. This migration is run on iOS 14 when launching an app whose WKWebView IndexedDB databases were created on iOS 13.

There's a flaw in the migration process that can result in multiple indexes being updated to have the same ID, and corrupting the associated IndexRecords data. Once this happens, it appears to be impossible to recover the correct indexes.

A detailed walkthrough of how this happens to the IDB indices in our app can be found here: https://gist.github.com/dpogue/53d529310355697a0bfdf17644a17840

It only happens in some pretty specific situations, but it's a pretty serious problem.

Comment 1 Sihui Liu 2020-09-25 12:21:27 PDT

Thanks for reporting! This is pretty bad since the corrupted records cannot be recovered.

Comment 2 Radar WebKit Bug Importer 2020-09-25 12:21:35 PDT

<rdar://problem/69587004>

Comment 3 Darryl Pogue 2020-09-25 13:09:38 PDT

Created attachment 409726 [details]
Testcase file

Here's a testcase file which can be used to demonstrate the problem.

I ran this in an iOS simulator on iOS 13.5 and opened it in Safari, then used `xcrun simctl upgrade` to upgrade the simulator to iOS 14.0 and opened it again. Looking at it with the Safari Inspector shows that one of the indexes is now missing.

Comment 4 Sihui Liu 2020-09-28 11:04:31 PDT

Created attachment 409900 [details]
Patch

Comment 5 Sihui Liu 2020-09-28 14:28:12 PDT

Created attachment 409924 [details]
Patch

Comment 6 Darin Adler 2020-09-28 14:54:41 PDT

Comment on attachment 409924 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=409924&action=review

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:242
> +static String indexInfoTableSchema(const String& tableName)

This argument should be ASCIILiteral or const char*, not const String&. It’s wastefully inefficient to turn an ASCII literal into a String and then destroy that string.

Comment 7 Darryl Pogue 2020-09-28 15:45:43 PDT

I tested this patch in both Safari and in my WKWebView-based app on the iOS simulator, and the corruption was fixed in both cases.
Thanks Sihui for jumping on this! :)

Comment 8 Sihui Liu 2020-09-28 16:28:21 PDT

Created attachment 409928 [details]
Patch

Comment 9 Sihui Liu 2020-09-28 16:32:06 PDT

(In reply to Darin Adler from comment #6)
> Comment on attachment 409924 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=409924&action=review
> 
> > Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:242
> > +static String indexInfoTableSchema(const String& tableName)
> 
> This argument should be ASCIILiteral or const char*, not const String&. It’s
> wastefully inefficient to turn an ASCII literal into a String and then
> destroy that string.

Right, changed to use ASCIILiteral.

Comment 10 EWS 2020-09-29 11:00:43 PDT

Committed r267753: <https://trac.webkit.org/changeset/267753>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 409928 [details].