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

@@ -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());

src/PreferencesDialog.ui

@@ -1268,6 +1268,16 @@ Can be set to 0 for disabling completion.</string>
          </property>
         </widget>
        </item>
+       <item>
+        <widget class="QCheckBox" name="checkAllowLoadExtension">
+         <property name="toolTip">
+          <string>&lt;html&gt;&lt;head/&gt;&lt;body&gt;&lt;p&gt;SQLite provides an SQL function for loading extensions from a shared library file. Activate this if you want to use the &lt;span style=&quot; font-style:italic;&quot;&gt;load_extension()&lt;/span&gt; function from SQL code.&lt;/p&gt;&lt;p&gt;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.&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</string>
+         </property>
+         <property name="text">
+          <string>Allow loading extensions from SQL code</string>
+         </property>
+        </widget>
+       </item>
       </layout>
      </widget>
      <widget class="QWidget" name="tabRemote">

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")
     {

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.")

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)
     {