[webkit-changes] [WebKit/WebKit] 9cfacd: Ensure SQLiteStorageArea does not use SQLiteStatem...

Rupin Mittal noreply at github.com
Wed Oct 30 21:23:54 PDT 2024


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 9cfacd819f89639031bcef018ee6955f0824a95f
      https://github.com/WebKit/WebKit/commit/9cfacd819f89639031bcef018ee6955f0824a95f
  Author: Rupin Mittal <rupin at apple.com>
  Date:   2024-10-30 (Wed, 30 Oct 2024)

  Changed paths:
    M Source/WebCore/platform/sql/SQLiteStatement.h
    M Source/WebCore/platform/sql/SQLiteStatementAutoResetScope.cpp
    M Source/WebCore/platform/sql/SQLiteStatementAutoResetScope.h
    M Source/WebKit/NetworkProcess/storage/SQLiteStorageArea.cpp

  Log Message:
  -----------
  Ensure SQLiteStorageArea does not use SQLiteStatement after it's been freed
https://bugs.webkit.org/show_bug.cgi?id=278538
rdar://127866416

Reviewed by Chris Dumez, Geoffrey Garen, and Sihui Liu.

SQLiteStorageArea::getItemFromDatabase() holds a SQLiteStatementAutoResetScope
statement variable. This goes out of scope at the end of getItemFromDatabase()
and so it's destructor is called. There is a crash occurring here. It turns out
that SQLiteStatementAutoResetScope holds a raw pointer to a SQLiteStatement.
A series of function calls: getItemFromDatabase() -> handleDatabaseErrorIfNeeded()
-> close() results in this SQLiteStatement object being destroyed
(SQLiteStorageArea holds a unique pointer to this SQLiteStatement object which is
destroyed in close()). After this destruction, the SQLiteStatementAutoResetScope
statement variable goes out of scope and it's destructor attempts to access the
raw pointer to the now-destroyed SQLiteStatement object. This use-after-free
causes the crash.

We fix this problem by using a block-scope to ensure the SQLiteStatementAutoResetScope
statement variable is destroyed before handleDatabaseErrorIfNeeded() is called. Although
the crash was only in getItemFromDatabase(), there are other functions in SQLiteStorageArea
that follow this same pattern and could potentially have a use-after-free. We make this
block-scope change there as well. Additionally, we change the SQLiteStatement raw pointer
in SQLiteStatementAutoResetScope to a CheckedPtr.

We use a block-scope rather than declare a new function SQLiteStatementAutoResetScope that
could call reset on the SQLiteStatement object because we want the object to live and be
reset in sync with the scope's lifetime.

* Source/WebCore/platform/sql/SQLiteStatement.h:
* Source/WebCore/platform/sql/SQLiteStatementAutoResetScope.cpp:
(WebCore::SQLiteStatementAutoResetScope::operator=): Deleted.
* Source/WebCore/platform/sql/SQLiteStatementAutoResetScope.h:
(WebCore::SQLiteStatementAutoResetScope::operator bool const):
(WebCore::SQLiteStatementAutoResetScope::get):
(WebCore::SQLiteStatementAutoResetScope::operator->):
* Source/WebKit/NetworkProcess/storage/SQLiteStorageArea.cpp:
(WebKit::SQLiteStorageArea::getItemFromDatabase):
(WebKit::SQLiteStorageArea::allItems):
(WebKit::SQLiteStorageArea::setItem):
(WebKit::SQLiteStorageArea::removeItem):
(WebKit::SQLiteStorageArea::clear):

Originally-landed-as: 280938.269 at safari-7619-branch (d76a8be1ba40). rdar://138875969
Canonical link: https://commits.webkit.org/285939@main



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications


More information about the webkit-changes mailing list