[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