Skip to content
Permalink
Browse files
HTTPSUpgradeList.db database should be opened in readonly mode
https://bugs.webkit.org/show_bug.cgi?id=195194
<rdar://problem/47103889>

Reviewed by Youenn Fablet.

Source/WebCore:

Add parameter to SQLiteDatabase::open() to specific the open flags.

* Modules/webdatabase/Database.cpp:
(WebCore::Database::performOpenAndVerify):
* platform/sql/SQLiteDatabase.cpp:
(WebCore::SQLiteDatabase::open):
* platform/sql/SQLiteDatabase.h:
* platform/sql/SQLiteFileSystem.cpp:
* platform/sql/SQLiteFileSystem.h:

Source/WebKit:

HTTPSUpgradeList.db database should be opened in readonly mode since it is not meant to be
modified by WebKit. Opening it in ReadWrite mode causes sandbox violations.

* NetworkProcess/NetworkHTTPSUpgradeChecker.cpp:
(WebKit::NetworkHTTPSUpgradeChecker::NetworkHTTPSUpgradeChecker):


Canonical link: https://commits.webkit.org/209529@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242251 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
cdumez committed Mar 1, 2019
1 parent 6fad189 commit c4d27cb61fcd2de443485e920a1605442115c9e0
Showing 8 changed files with 52 additions and 20 deletions.
@@ -1,3 +1,21 @@
2019-02-28 Chris Dumez <cdumez@apple.com>

HTTPSUpgradeList.db database should be opened in readonly mode
https://bugs.webkit.org/show_bug.cgi?id=195194
<rdar://problem/47103889>

Reviewed by Youenn Fablet.

Add parameter to SQLiteDatabase::open() to specific the open flags.

* Modules/webdatabase/Database.cpp:
(WebCore::Database::performOpenAndVerify):
* platform/sql/SQLiteDatabase.cpp:
(WebCore::SQLiteDatabase::open):
* platform/sql/SQLiteDatabase.h:
* platform/sql/SQLiteFileSystem.cpp:
* platform/sql/SQLiteFileSystem.h:

2019-02-28 Brady Eidson <beidson@apple.com>

Followup to:
@@ -348,7 +348,7 @@ ExceptionOr<void> Database::performOpenAndVerify(bool shouldSetVersionInNewDatab

SQLiteTransactionInProgressAutoCounter transactionCounter;

if (!m_sqliteDatabase.open(m_filename, true))
if (!m_sqliteDatabase.open(m_filename))
return Exception { InvalidStateError, formatErrorMessage("unable to open database", m_sqliteDatabase.lastError(), m_sqliteDatabase.lastErrorMsg()) };
if (!m_sqliteDatabase.turnOnIncrementalAutoVacuum())
LOG_ERROR("Unable to turn on incremental auto-vacuum (%d %s)", m_sqliteDatabase.lastError(), m_sqliteDatabase.lastErrorMsg());
@@ -35,6 +35,7 @@
#include <mutex>
#include <sqlite3.h>
#include <thread>
#include <wtf/FileSystem.h>
#include <wtf/Threading.h>
#include <wtf/text/CString.h>
#include <wtf/text/StringConcatenateNumbers.h>
@@ -76,13 +77,26 @@ SQLiteDatabase::~SQLiteDatabase()
close();
}

bool SQLiteDatabase::open(const String& filename, bool forWebSQLDatabase)
bool SQLiteDatabase::open(const String& filename, OpenMode openMode)
{
initializeSQLiteIfNecessary();

close();

m_openError = SQLiteFileSystem::openDatabase(filename, &m_db, forWebSQLDatabase);
int flags = SQLITE_OPEN_AUTOPROXY;
switch (openMode) {
case OpenMode::ReadOnly:
flags |= SQLITE_OPEN_READONLY;
break;
case OpenMode::ReadWrite:
flags |= SQLITE_OPEN_READWRITE;
break;
case OpenMode::ReadWriteCreate:
flags |= SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE;
break;
}

m_openError = sqlite3_open_v2(FileSystem::fileSystemRepresentation(filename).data(), &m_db, flags, nullptr);
if (m_openError != SQLITE_OK) {
m_openErrorMessage = m_db ? sqlite3_errmsg(m_db) : "sqlite_open returned null";
LOG_ERROR("SQLite database failed to load from %s\nCause - %s", filename.ascii().data(),
@@ -53,7 +53,8 @@ class SQLiteDatabase {
WEBCORE_EXPORT SQLiteDatabase();
WEBCORE_EXPORT ~SQLiteDatabase();

WEBCORE_EXPORT bool open(const String& filename, bool forWebSQLDatabase = false);
enum class OpenMode { ReadOnly, ReadWrite, ReadWriteCreate };
WEBCORE_EXPORT bool open(const String& filename, OpenMode = OpenMode::ReadWriteCreate);
bool isOpen() const { return m_db; }
WEBCORE_EXPORT void close();

@@ -46,11 +46,6 @@ SQLiteFileSystem::SQLiteFileSystem()
{
}

int SQLiteFileSystem::openDatabase(const String& filename, sqlite3** database, bool)
{
return sqlite3_open_v2(FileSystem::fileSystemRepresentation(filename).data(), database, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_AUTOPROXY, nullptr);
}

String SQLiteFileSystem::appendDatabaseFileNameToPath(const String& path, const String& fileName)
{
return FileSystem::pathByAppendingComponent(path, fileName);
@@ -44,16 +44,6 @@ class SQLiteDatabase;
// by the WebKit database code.
class SQLiteFileSystem {
public:
// Opens a database file.
//
// filemame - The name of the database file.
// database - The SQLite structure that represents the database stored
// in the given file.
// forWebSQLDatabase - True, if and only if we're opening a Web SQL Database file.
// Used by Chromium to determine if the DB file needs to be opened
// using a custom VFS.
static int openDatabase(const String& filename, sqlite3** database, bool forWebSQLDatabase);

// Creates an absolute file path given a directory and a file name.
//
// path - The directory.
@@ -1,3 +1,17 @@
2019-02-28 Chris Dumez <cdumez@apple.com>

HTTPSUpgradeList.db database should be opened in readonly mode
https://bugs.webkit.org/show_bug.cgi?id=195194
<rdar://problem/47103889>

Reviewed by Youenn Fablet.

HTTPSUpgradeList.db database should be opened in readonly mode since it is not meant to be
modified by WebKit. Opening it in ReadWrite mode causes sandbox violations.

* NetworkProcess/NetworkHTTPSUpgradeChecker.cpp:
(WebKit::NetworkHTTPSUpgradeChecker::NetworkHTTPSUpgradeChecker):

2019-02-28 David Quesada <david_quesada@apple.com>

Expose APINavigationAction.shouldPerformDownload() on WKNavigationAction
@@ -68,7 +68,7 @@ NetworkHTTPSUpgradeChecker::NetworkHTTPSUpgradeChecker()
return;
}

bool isDatabaseOpen = m_database->open(path);
bool isDatabaseOpen = m_database->open(path, WebCore::SQLiteDatabase::OpenMode::ReadOnly);
if (!isDatabaseOpen) {
#if PLATFORM(COCOA)
RELEASE_LOG_ERROR(Network, "%p - NetworkHTTPSUpgradeChecker::open failed, error message: %{public}s, database path: %{public}s", this, m_database->lastErrorMsg(), path.utf8().data());

0 comments on commit c4d27cb

Please sign in to comment.