Closed Bug 1619522 (SQLite3.32.1) Opened 4 years ago Closed 4 years ago

Upgrade to SQLite 3.32.1

Categories

(Toolkit :: Storage, task, P3)

Product:
Toolkit
Component:
Storage
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla79
Tracking Flags:
Tracking Status
firefox79 --- fixed

People

(Reporter: mak, Assigned: RyanVM)

References

(Blocks 1 open bug,
URL
)

Details

Attachments

(1 file)

Bug 1619522 - Upgrade to SQLite 3.32.1. r=mak
47 bytes, text/x-phabricator-request
Details | Review

2020-05-25 (3.32.1)

Fix two long-standing bugs that allow malicious SQL statements to crash the process that is running SQLite. These bugs were announced by a third-party approximately 24 hours after the 3.32.0 release but are not specific to the 3.32.0 release.
Other minor compiler-warning fixes and whatnot.

Hashes:
SQLITE_SOURCE_ID: 2020-05-25 16:19:56 0c1fcf4711a2e66c813aed38cf41cd3e2123ee8eb6db98118086764c4ba83350
SHA3-256 for sqlite3.c: f695ae21abf045e4ee77980a67ab2c6e03275009e593ee860a2eabf840482372

2020-05-22 (3.32.0)

Added support for approximate ANALYZE using the PRAGMA analysis_limit command.
Added the bytecode virtual table.
Add the checksum VFS shim to the set of run-time loadable extensions included in the source tree.
Added the iif() SQL function.
INSERT and UPDATE statements now always apply column affinity before computing CHECK constraints. This bug fix could, in theory, cause problems for legacy databases with unorthodox CHECK constraints the require the input type for an INSERT is different from the declared column type. See ticket 86ba67afafded936 for more information.
Added the sqlite3_create_filename(), sqlite3_free_filename(), and sqlite3_database_file_object() interfaces to better support of VFS shim implementations.
Increase the default upper bound on the number of parameters from 999 to 32766.
Added code for the UINT collating sequence as an optional loadable extension.
Enhancements to the CLI:
    Add options to the .import command: --csv, --ascii, --skip
    The .dump command now accepts multiple LIKE-pattern arguments and outputs the union of all matching tables.
    Add the .oom command in debugging builds
    Add the --bom option to the .excel, .output, and .once commands.
    Enhance the .filectrl command to support the --schema option.
    The UINT collating sequence extension is automatically loaded 
The ESCAPE clause of a LIKE operator now overrides wildcard characters, so that the behavior now matches what PostgreSQL does.
SQLITE_SOURCE_ID: 2020-05-22 17:46:16 5998789c9c744bce92e4cff7636bba800a75574243d6977e1fc8281e360f8d5a
SHA3-256 for sqlite3.c: 33ed868b21b62ce1d0352ed88bdbd9880a42f29046497a222df6459fc32a356f
Alias: SQLite3.32 → SQLite3.32.0

FYI, I ran the latest snapshot build (sqlite-snapshot-202003121754) through Try and there's new failures in test_storage_connection.js.

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=293702408&repo=try&lineNumber=4359

WARNING -  TEST-UNEXPECTED-FAIL | storage/test/unit/test_storage_connection.js | test_variableLimit - [test_variableLimit : 1015] Should return default limit - 32766 == 999
  INFO -  /builds/worker/workspace/build/tests/xpcshell/tests/storage/test/unit/test_storage_connection.js:test_variableLimit:1015
  INFO -  /builds/worker/workspace/build/tests/xpcshell/head.js:_run_next_test/<:1567
  INFO -  /builds/worker/workspace/build/tests/xpcshell/head.js:_run_next_test:1567
  INFO -  /builds/worker/workspace/build/tests/xpcshell/head.js:run:735
  INFO -  /builds/worker/workspace/build/tests/xpcshell/head.js:_do_main:246
  INFO -  /builds/worker/workspace/build/tests/xpcshell/head.js:_execute_test:573
  INFO -  -e:null:1
  INFO -  exiting test
  INFO -  (xpcshell/head.js) | test run_next_test 44 finished (2)
  INFO -  Unexpected exception NS_ERROR_ABORT:
  INFO -  _abort_failed_test@/builds/worker/workspace/build/tests/xpcshell/head.js:791:20
  INFO -  do_report_result@/builds/worker/workspace/build/tests/xpcshell/head.js:892:5
  INFO -  Assert<@/builds/worker/workspace/build/tests/xpcshell/head.js:67:21
  INFO -  proto.report@resource://testing-common/Assert.jsm:233:10
  INFO -  equal@resource://testing-common/Assert.jsm:275:8
  INFO -  test_variableLimit@/builds/worker/workspace/build/tests/xpcshell/tests/storage/test/unit/test_storage_connection.js:1015:10
  INFO -  _run_next_test/<@/builds/worker/workspace/build/tests/xpcshell/head.js:1567:22
  INFO -  _run_next_test@/builds/worker/workspace/build/tests/xpcshell/head.js:1567:38
  INFO -  run@/builds/worker/workspace/build/tests/xpcshell/head.js:735:9
  INFO -  _do_main@/builds/worker/workspace/build/tests/xpcshell/head.js:246:6
  INFO -  _execute_test@/builds/worker/workspace/build/tests/xpcshell/head.js:573:5
  INFO -  @-e:1:1
  INFO -  exiting test
  INFO -  <<<<<<<
Summary: Upgrade to SQLite 3.32 → Upgrade to SQLite 3.32.0

yeah, it's due to this change: "Increase the default upper bound on the number of parameters from 999 to 32766."
We must update the test to not hardcode a number.

Wouldn't we want to just update the test to the new upper bound? I'm confused what it's testing if we aren't hard-coding a number there (since IIUC, we're sanity-checking the number?)

Latest snapshot is green on Try with the test expectation updated to the new value:
https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=f1f20a72e5a7cce537f7227ca66a8379eeda83af

(In reply to Ryan VanderMeulen [:RyanVM] from comment #3)

Wouldn't we want to just update the test to the new upper bound? I'm confused what it's testing if we aren't hard-coding a number there (since IIUC, we're sanity-checking the number?)

yeah, it's a sanity check, I was hoping to get the value directly from Sqlite but... doesn't seem worth it. what you did is fine.

I observed locally and on try that a recent SQLite 3.32.0 snapshot (2020-05-17 13:47:28 69e149f76853d196c8855fedfc98848b60fb116ac36bc08824b1a122469f8ece) seems to break ./mach wpt /IndexedDB/idbtransaction_objectStoreNames.html.

Reproducible on Try with the latest 2020-05-19 snapshot:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=302945776&repo=try&lineNumber=6282

Andrew, I feel like we've seen issues like this before. Does this ring a bell to you? Looks sorta-similar to the issues we had with the 3.31 update?

Flags: needinfo?(bugmail)

This seems like a new and exciting issue to me.

Flags: needinfo?(bugmail)

Simon, it sounds like you might already be looking into this, or closest to looking into this... :)

Flags: needinfo?(sgiesecke)

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #10)

Simon, it sounds like you might already be looking into this, or closest to looking into this... :)

Yes, I will give this a closer look.

There is an object store with name 0xFFFD (UTF-16 replacement character), and then an object store with name 0xDC00 (UTF-16 surrogate trail) is tried to be created. Then the query at https://searchfox.org/mozilla-central/rev/61fceb7c0729773f544a9656f474e36cd636e5ea/dom/indexedDB/ActorsParent.cpp#23633 matches these two names as being identical.

I am pretty sure this is caused by https://www.sqlite.org/src/info/7fab1393c2b22b1f:

Convert invalid surrogates to 0xfffd when translating UTF.

The same problem is with 0xD800 (UTF-16 surrogate trail). When removing/commenting out https://searchfox.org/mozilla-central/rev/61fceb7c0729773f544a9656f474e36cd636e5ea/testing/web-platform/tests/IndexedDB/idbtransaction_objectStoreNames.html#147-148, the test is fine.

I see the following options to proceed with this:

  1. Work around the change in SQLite. This probably does not only affect object store names but any UTF-16 data, so working around that would require transforming all strings...
  2. Suggest reverting the change in SQLite.
  3. Suggest removing these test cases or maybe change them into rejecting these characters as object store names (and adapt our implementation accordingly).
Flags: needinfo?(sgiesecke)

I am exploring the option of backing out the SQLite changes and to do a better job of testing and documenting the fact that SQLite preserves invalid surrogate pairs in UTF content. I am working on those changes now. I will report my findings later today.

The change has now been backed out. The latest Prerelease Snapshot (available at https://sqlite.org/download.html) should contain the fix. Please let us know if you find otherwise. We are expecting to cut the final release soon.

Thanks, I'll give it a spin later today!

Alias: SQLite3.32.0 → SQLite3.32.1
URL: https://www.sqlite.org/releaselog/3_3...
Summary: Upgrade to SQLite 3.32.0 → Upgrade to SQLite 3.32.1
Assignee: nobody → ryanvm

https://hg.mozilla.org/mozilla-central/rev/b4bd43bce2be

Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox79: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla79

I have nothing against taking the update for 79, it's minor fixes anyway.

Blocks: SQLite3.32.3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: