From de9874d324b3ae6b02f68a74558dc824836e3df6 Mon Sep 17 00:00:00 2001 From: Manuel Date: Wed, 10 Oct 2018 21:26:59 +0200 Subject: [PATCH] Leaving the loading of extensions enabled might be a security risk (#1558) * Leaving the loading of extensions enabled might be a security risk Using sqlite3_enable_load_extension not only allows loading extensions through the C-API but also through the SQL functioon load_extension(). That might be a security risk if the user is unaware that executing an SQL file can lead to native code execution and not only to database file modification. See issue #1551 * Preference for allowing loading extensions from SQL code New setting that authorizes the execution of load_extension() from SQL code. Default value, false, following the design decision of SQLite, that disables this function unless by default. Added notice about the option in the calltips of the two function variants. --- src/PreferencesDialog.cpp | 2 ++ src/PreferencesDialog.ui | 10 ++++++++++ src/Settings.cpp | 4 ++++ src/SqlUiLexer.cpp | 4 ++-- src/sqlitedb.cpp | 20 +++++++++++++------- 5 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/PreferencesDialog.cpp b/src/PreferencesDialog.cpp index 2ac3ad4965..bcbd322382 100644 --- a/src/PreferencesDialog.cpp +++ b/src/PreferencesDialog.cpp @@ -181,6 +181,7 @@ void PreferencesDialog::loadSettings() ui->listExtensions->addItems(Settings::getValue("extensions", "list").toStringList()); ui->checkRegexDisabled->setChecked(Settings::getValue("extensions", "disableregex").toBool()); + ui->checkAllowLoadExtension->setChecked(Settings::getValue("extensions", "enable_load_extension").toBool()); fillLanguageBox(); ui->toolbarStyleComboBox->setCurrentIndex(Settings::getValue("General", "toolbarStyle").toInt()); } @@ -240,6 +241,7 @@ void PreferencesDialog::saveSettings() extList.append(item->text()); Settings::setValue("extensions", "list", extList); Settings::setValue("extensions", "disableregex", ui->checkRegexDisabled->isChecked()); + Settings::setValue("extensions", "enable_load_extension", ui->checkAllowLoadExtension->isChecked()); // Save remote settings Settings::setValue("remote", "active", ui->checkUseRemotes->isChecked()); diff --git a/src/PreferencesDialog.ui b/src/PreferencesDialog.ui index b5f6d1f0d2..f603f70b9b 100644 --- a/src/PreferencesDialog.ui +++ b/src/PreferencesDialog.ui @@ -1268,6 +1268,16 @@ Can be set to 0 for disabling completion. + + + + <html><head/><body><p>SQLite provides an SQL function for loading extensions from a shared library file. Activate this if you want to use the <span style=" font-style:italic;">load_extension()</span> function from SQL code.</p><p>For security reasons, extension loading is turned off by default and must be enabled through this setting. You can always load extensions through the GUI, even though this option is disabled.</p></body></html> + + + Allow loading extensions from SQL code + + + diff --git a/src/Settings.cpp b/src/Settings.cpp index a0f77f5466..6b1ecf98fd 100644 --- a/src/Settings.cpp +++ b/src/Settings.cpp @@ -326,6 +326,10 @@ QVariant Settings::getDefaultValue(const QString& group, const QString& name) if(group == "extension" && name == "disableregex") return false; + // extensions/enable_load_extension? + if(group == "extension" && name == "enable_load_extension") + return false; + // PlotDock/lineType or pointShape? if(group == "PlotDock") { diff --git a/src/SqlUiLexer.cpp b/src/SqlUiLexer.cpp index b10d75eb39..e050eb714f 100644 --- a/src/SqlUiLexer.cpp +++ b/src/SqlUiLexer.cpp @@ -76,8 +76,8 @@ void SqlUiLexer::setupAutoCompletion() << "length" + tr("(X) For a string value X, the length(X) function returns the number of characters (not bytes) in X prior to the first NUL character.") << "like" + tr("(X,Y) The like() function is used to implement the \"Y LIKE X\" expression.") << "like" + tr("(X,Y,Z) The like() function is used to implement the \"Y LIKE X ESCAPE Z\" expression.") - << "load_extension" + tr("(X) The load_extension(X) function loads SQLite extensions out of the shared library file named X.") - << "load_extension" + tr("(X,Y) The load_extension(X) function loads SQLite extensions out of the shared library file named X using the entry point Y.") + << "load_extension" + tr("(X) The load_extension(X) function loads SQLite extensions out of the shared library file named X.\nUse of this function must be authorized from Preferences.") + << "load_extension" + tr("(X,Y) The load_extension(X) function loads SQLite extensions out of the shared library file named X using the entry point Y.\nUse of this function must be authorized from Preferences.") << "lower" + tr("(X) The lower(X) function returns a copy of string X with all ASCII characters converted to lower case.") << "ltrim" + tr("(X) ltrim(X) removes spaces from the left side of X.") << "ltrim" + tr("(X,Y) The ltrim(X,Y) function returns a string formed by removing any and all characters that appear in Y from the left side of X.") diff --git a/src/sqlitedb.cpp b/src/sqlitedb.cpp index bbffd15426..49fe84b6fc 100644 --- a/src/sqlitedb.cpp +++ b/src/sqlitedb.cpp @@ -147,9 +147,6 @@ bool DBBrowserDB::open(const QString& db, bool readOnly) bool foreignkeys = Settings::getValue("db", "foreignkeys").toBool(); setPragma("foreign_keys", foreignkeys ? "1" : "0"); - // Enable extension loading - sqlite3_enable_load_extension(_db, 1); - // Register REGEXP function if(Settings::getValue("extensions", "disableregex").toBool() == false) sqlite3_create_function(_db, "REGEXP", 2, SQLITE_UTF8, nullptr, regexp, nullptr, nullptr); @@ -523,9 +520,6 @@ bool DBBrowserDB::create ( const QString & db) bool foreignkeys = Settings::getValue("db", "foreignkeys").toBool(); setPragma("foreign_keys", foreignkeys ? "1" : "0"); - // Enable extension loading - sqlite3_enable_load_extension(_db, 1); - // Register REGEXP function if(Settings::getValue("extensions", "disableregex").toBool() == false) sqlite3_create_function(_db, "REGEXP", 2, SQLITE_UTF8, nullptr, regexp, nullptr, nullptr); @@ -1792,9 +1786,19 @@ bool DBBrowserDB::loadExtension(const QString& filePath) return false; } + // Enable extension loading + sqlite3_enable_load_extension(_db, 1); + // Try to load extension char* error; - if(sqlite3_load_extension(_db, filePath.toUtf8(), nullptr, &error) == SQLITE_OK) + int result = sqlite3_load_extension(_db, filePath.toUtf8(), nullptr, &error); + + // Disable extension loading if so configured + // (we don't want to leave the possibility of calling load_extension() from SQL without user informed permission) + if (!Settings::getValue("extensions", "enable_load_extension").toBool()) + sqlite3_enable_load_extension(_db, 0); + + if (result == SQLITE_OK) { return true; } else { @@ -1810,6 +1814,8 @@ void DBBrowserDB::loadExtensionsFromSettings() if(!_db) return; + sqlite3_enable_load_extension(_db, Settings::getValue("extensions", "enable_load_extension").toBool()); + QStringList list = Settings::getValue("extensions", "list").toStringList(); for(const QString& ext : list) {