bug#63590: 29.0.90; can't load sqlite extension

bug#63590: 29.0.90; can't load sqlite extension

[Lennart Vogelsang via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dear maintainers,

I am experimenting around loading sqlite extensions into the builtin
sqlite capability of emacs. Sadly I do not seem to be able to load any
of csv extensions, even when their name appears in the hard coded
allow list in sqlite.c.

To reproduce, I've created an empty folder, cd'ed into it, started
emacs -Q, copied the sqlite's csv extension source code [0] into 
csvtable.c,
compiled it with

      gcc -O3 -Wall -Wno-unknown-pragmas -fPIC -shared -lm -o 
csvtable.so csvtable.c

and executed the following elisp forms in the scratch buffer:

      (setq-local mydb (sqlite-open))
      (sqlite-load-extension mydb "./csvtable.so")

I get a nil return value from the second expression, indicating
that it did not load the extension (verified by using the `csv` module
in a `sqlite-execute` call). If I try the same from the `sqlite3` cli
interface, it works:

      .load ./csvtable.so

Emacs's sqlite implementation does not setup the load extension config
(SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION [1]) anywhere as far as I could
gather, so I suspect that to be the root cause of the issue (the sqlite3
cli sets that configuration). It has to be setup with the `use_config`
[2] c function, e.g.

sqlite3_db_config(db,SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,1,NULL)

before one can load extensions via sqlite's c interface
`sqlite3_load_extension()` that emacs also uses [3].

For testing, I've tried compiling emacs with a small change
that adds the sqlite3_db_config call to the sqlite-open function, and
with this change, the above procedure works as expected.

If someone with more experience and knowledge could confirm that this is
indeed the issue and that a patch is wanted, I am happy to make a small
patch that adds the configuration to the sqlite-open function of
sqlite.c (or would there be a better position to add that config
setting?).

Happy greetings,
Lennart

Additional sources:

[0] https://www.sqlite.org/src/artifact?ci=trunk&filename=ext/misc/csv.c
[1] 
https://www.sqlite.org/c3ref/c_dbconfig_defensive.html#sqlitedbconfigenableloadextension
[2] https://www.sqlite.org/c3ref/db_config.html
[3] https://www.sqlite.org/loadext.html

In GNU Emacs 29.0.90 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.24.37, cairo version 1.17.8) of 2023-04-27 built on Jupiter
Repository revision: c46e93b1f50c9a6f7143f347d96a6385bcdf3a05
Repository branch: emacs-29
System Description: Arch Linux

Configured using:
'configure --with-pgtk --with-treesitter'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY
PDUMPER PGTK PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP XIM GTK3 ZLIB

Important settings:
value of $LANG: en_US.UTF-8
value of $XMODIFIERS: @im=ibus
locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
tooltip-mode: t
global-eldoc-mode: t
eldoc-mode: t
show-paren-mode: t
electric-indent-mode: t
mouse-wheel-mode: t
tool-bar-mode: t
menu-bar-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
font-lock-mode: t
blink-cursor-mode: t
line-number-mode: t
indent-tabs-mode: t
transient-mark-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils time-date subr-x
cl-loaddefs cl-lib rmc iso-transl tooltip cconv eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/pgtk-win pgtk-win term/common-win pgtk-dnd tool-bar dnd fontset
image regexp-opt fringe tabulated-list replace newcomment text-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify
dynamic-setting system-font-setting font-render-setting cairo gtk pgtk
lcms2 multi-tty make-network-process emacs)

Memory information:
((conses 16 36722 8121)
(symbols 48 5117 0)
(strings 32 13127 1970)
(string-bytes 1 373207)
(vectors 16 9308)
(vector-slots 8 148657 12041)
(floats 8 22 22)
(intervals 56 310 0)
(buffers 984 11))

bug#63590: 29.0.90; can't load sqlite extension

[Eli Zaretskii]

> Date: Fri, 19 May 2023 15:25:21 +0200
> From:  Lennart Vogelsang via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> I am experimenting around loading sqlite extensions into the builtin
> sqlite capability of emacs. Sadly I do not seem to be able to load any
> of csv extensions, even when their name appears in the hard coded
> allow list in sqlite.c.
> 
> To reproduce, I've created an empty folder, cd'ed into it, started
> emacs -Q, copied the sqlite's csv extension source code [0] into 
> csvtable.c,
> compiled it with
> 
>       gcc -O3 -Wall -Wno-unknown-pragmas -fPIC -shared -lm -o 
> csvtable.so csvtable.c
> 
> and executed the following elisp forms in the scratch buffer:
> 
>       (setq-local mydb (sqlite-open))
>       (sqlite-load-extension mydb "./csvtable.so")
> 
> I get a nil return value from the second expression, indicating
> that it did not load the extension (verified by using the `csv` module
> in a `sqlite-execute` call). If I try the same from the `sqlite3` cli
> interface, it works:
> 
>       .load ./csvtable.so
> 
> Emacs's sqlite implementation does not setup the load extension config
> (SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION [1]) anywhere as far as I could
> gather, so I suspect that to be the root cause of the issue (the sqlite3
> cli sets that configuration). It has to be setup with the `use_config`
> [2] c function, e.g.
> 
> sqlite3_db_config(db,SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,1,NULL)
> 
> before one can load extensions via sqlite's c interface
> `sqlite3_load_extension()` that emacs also uses [3].
> 
> For testing, I've tried compiling emacs with a small change
> that adds the sqlite3_db_config call to the sqlite-open function, and
> with this change, the above procedure works as expected.

Are you saying that the SQL-related tests in the Emacs test suite
don't work for you?  See test/src/sqlite-tests.el.

These tests were used to test the Emacs built-in SQL support, and I'm
sure they worked when Lars, who developed that, worked on it.  So how
come this doesn't work for you?

bug#63590: 29.0.90; can't load sqlite extension

[Eli Zaretskii]

> Cc: 63590@debbugs.gnu.org
> Date: Fri, 19 May 2023 19:36:21 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> Are you saying that the SQL-related tests in the Emacs test suite
> don't work for you?  See test/src/sqlite-tests.el.

Answering myself: the relevant test considers it a "success" if
sqlite-load-extension returns nil, even if the extension does exist on
the system.  So it doesn't really test whether the extension was
successfully loaded.  I will tweak the test to be more useful in this
regard.

Anyway, does the patch to sqlite.c below fix your problem?  I decided
to enable loading extensions only temporarily, while we call
sqlite3_load_extension, so that no extension could be accidentally
loaded out of our control, not even if some Emacs module uses the
sqlite3 library on its own using the C APIs, thus bypassing
sqlite-load-extension.

diff --git a/src/sqlite.c b/src/sqlite.c
index 0361514..2b0bc02 100644
--- a/src/sqlite.c
+++ b/src/sqlite.c
@@ -23,6 +23,8 @@ Copyright (C) 2021-2023 Free Software Foundation, Inc.
    https://github.com/syohex/emacs-sqlite3  */
 
 #include <config.h>
+
+#include <c-strcase.h>
 #include "lisp.h"
 #include "coding.h"
 
@@ -686,7 +692,8 @@ DEFUN ("sqlite-load-extension", Fsqlite_load_extension,
   /* Add names of useful and free modules here.  */
   const char *allowlist[3] = { "pcre", "csvtable", NULL };
   char *name = SSDATA (Ffile_name_nondirectory (module));
-  /* Possibly skip past a common prefix.  */
+  /* Possibly skip past a common prefix (libsqlite3_mod_ is used by
+     Debian, see https://packages.debian.org/source/sid/sqliteodbc).  */
   const char *prefix = "libsqlite3_mod_";
   if (!strncmp (name, prefix, strlen (prefix)))
     name += strlen (prefix);
@@ -697,7 +704,7 @@ DEFUN ("sqlite-load-extension", Fsqlite_load_extension,
       if (strlen (*allow) < strlen (name)
 	  && !strncmp (*allow, name, strlen (*allow))
 	  && (!strcmp (name + strlen (*allow), ".so")
-	      || !strcmp (name + strlen (*allow), ".DLL")))
+	      || !strcasecmp (name + strlen (*allow), ".dll")))
 	{
 	  do_allow = true;
 	  break;
@@ -707,12 +714,25 @@ DEFUN ("sqlite-load-extension", Fsqlite_load_extension,
   if (!do_allow)
     xsignal1 (Qsqlite_error, build_string ("Module name not on allowlist"));
 
-  int result = sqlite3_load_extension
-		       (XSQLITE (db)->db,
-			SSDATA (ENCODE_FILE (Fexpand_file_name (module, Qnil))),
-			NULL, NULL);
-  if (result ==  SQLITE_OK)
-    return Qt;
+  /* Expand all Lisp data explicitly, so as to avoid signaling an
+     error while extension loading is enabled -- we don't want to
+     "leak" this outside this function.  */
+  sqlite3 *sdb = XSQLITE (db)->db;
+  char *ext_fn = SSDATA (ENCODE_FILE (Fexpand_file_name (module, Qnil)));
+  /* Temporarily enable loading extensions via the C API.  */
+  int result = sqlite3_db_config (sdb, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 1,
+				  NULL);
+  if (result == SQLITE_OK)
+    {
+      result = sqlite3_load_extension (sdb, ext_fn, NULL, NULL);
+      if (result ==  SQLITE_OK)
+	{
+	  /* Disable loading extensions via C API.  */
+	  sqlite3_db_config (sdb, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 0,
+			     NULL);
+	  return Qt;
+	}
+    }
   return Qnil;
 }
 #endif /* HAVE_SQLITE3_LOAD_EXTENSION */

bug#63590: 29.0.90; can't load sqlite extension

[Eli Zaretskii]

> Date: Fri, 19 May 2023 15:25:21 +0200
> From:  Lennart Vogelsang via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> To reproduce, I've created an empty folder, cd'ed into it, started
> emacs -Q, copied the sqlite's csv extension source code [0] into 
> csvtable.c,
> compiled it with
> 
>       gcc -O3 -Wall -Wno-unknown-pragmas -fPIC -shared -lm -o 
> csvtable.so csvtable.c
> 
> and executed the following elisp forms in the scratch buffer:
> 
>       (setq-local mydb (sqlite-open))
>       (sqlite-load-extension mydb "./csvtable.so")
> 
> I get a nil return value from the second expression, indicating
> that it did not load the extension (verified by using the `csv` module
> in a `sqlite-execute` call). If I try the same from the `sqlite3` cli
> interface, it works:
> 
>       .load ./csvtable.so

I think you made one more change to csv.c: you renamed the function
sqlite3_csv_init to the name sqlite3_csvtable_init.  Otherwise, the
loading would fail, because sqlite3's cli will not find the entry
function it expects.

More importantly: the csv.c source file to which you point, viz.:

  https://www.sqlite.org/src/artifact?ci=trunk&filename=ext/misc/csv.c

is NOT the source file of the libsqlite3_mod_csvtable.so extension
distributed by Debian, which we currently have on the "allow list", it
is a different extension.  The source of csvtable is here:

  https://packages.debian.org/sid/libsqlite3-mod-csvtable

bug#63590: 29.0.90; can't load sqlite extension

[Lennart Vogelsang via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Ahh, I just wanted to answer you, I just noticed that about the tests 
too. Thank you! Your patch works for me,
just one small thing: sqlite extension loading can also fail because of 
other reasons (e.g. if the shared library
does not exist). Currently your patch would leave sqlite extension 
loading enabled in that case, I think?

I would also argue that it would make sense to actually report the error 
of the extension loading (when  the dynamic library file does not exist, 
or the extension is invalid). Maybe something like this:

diff --git a/src/sqlite.c b/src/sqlite.c
index 0361514766a..4be8acc9a94 100644
--- a/src/sqlite.c
+++ b/src/sqlite.c
@@ -23,6 +23,8 @@ Copyright (C) 2021-2023 Free Software Foundation, Inc.
     https://github.com/syohex/emacs-sqlite3  */

  #include <config.h>
+
+#include <c-strcase.h>
  #include "lisp.h"
  #include "coding.h"

@@ -686,7 +688,8 @@ DEFUN ("sqlite-load-extension", Fsqlite_load_extension,
    /* Add names of useful and free modules here.  */
    const char *allowlist[3] = { "pcre", "csvtable", NULL };
    char *name = SSDATA (Ffile_name_nondirectory (module));
-  /* Possibly skip past a common prefix.  */
+  /* Possibly skip past a common prefix (libsqlite3_mod_ is used by
+     Debian, see https://packages.debian.org/source/sid/sqliteodbc).  */
    const char *prefix = "libsqlite3_mod_";
    if (!strncmp (name, prefix, strlen (prefix)))
      name += strlen (prefix);
@@ -697,7 +700,7 @@ DEFUN ("sqlite-load-extension", Fsqlite_load_extension,
        if (strlen (*allow) < strlen (name)
        && !strncmp (*allow, name, strlen (*allow))
        && (!strcmp (name + strlen (*allow), ".so")
-          || !strcmp (name + strlen (*allow), ".DLL")))
+          || !strcasecmp (name + strlen (*allow), ".dll")))
      {
        do_allow = true;
        break;
@@ -707,12 +710,32 @@ DEFUN ("sqlite-load-extension", 
Fsqlite_load_extension,
    if (!do_allow)
      xsignal1 (Qsqlite_error, build_string ("Module name not on 
allowlist"));

-  int result = sqlite3_load_extension
-               (XSQLITE (db)->db,
-            SSDATA (ENCODE_FILE (Fexpand_file_name (module, Qnil))),
-            NULL, NULL);
-  if (result ==  SQLITE_OK)
-    return Qt;
+  /* Expand all Lisp data explicitly, so as to avoid signaling an
+     error while extension loading is enabled -- we don't want to
+     "leak" this outside this function.  */
+  sqlite3 *sdb = XSQLITE (db)->db;
+  char *ext_fn = SSDATA (ENCODE_FILE (Fexpand_file_name (module, Qnil)));
+  /* Temporarily enable loading extensions via the C API.  */
+  int result = sqlite3_db_config (sdb, 
SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 1,
+                  NULL);
+  if (result == SQLITE_OK)
+    {
+      /* save error from sqlite */
+      char *errmsg;
+      result = sqlite3_load_extension (sdb, ext_fn, NULL, &errmsg);
+      /* Disable loading extensions via C API.  */
+      sqlite3_db_config (sdb, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION,
+             0, NULL);
+      if (result == SQLITE_OK)
+    {
+      return Qt;
+    }
+      else
+    {
+      xsignal1 (Qsqlite_error, build_string (errmsg));
+      sqlite_free (errmsg);
+    }
+    }
    return Qnil;
  }
  #endif /* HAVE_SQLITE3_LOAD_EXTENSION */

That way, the test also correctly fails as we signal the error from the 
extension loading.

Regarding csv.c, yes I forgot to mention that. I admit for testing 
purposes I changed the name there (to sqlite3_extension_init, which 
sqlite also always accepts). Thank you for pointing me to the real 
extension. Just out of curiosity, as there are a handful of useful 
sqlite extensions out there, could there be a way to make the allow list 
a bit more lenient? Maybe as a build configure feature allowing us to 
specify other extensions that are allowed to be loaded.

On 5/20/23 11:59 AM, Eli Zaretskii wrote:
>> Date: Fri, 19 May 2023 15:25:21 +0200
>> From:  Lennart Vogelsang via "Bug reports for GNU Emacs,
>>   the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>>
>> To reproduce, I've created an empty folder, cd'ed into it, started
>> emacs -Q, copied the sqlite's csv extension source code [0] into
>> csvtable.c,
>> compiled it with
>>
>>        gcc -O3 -Wall -Wno-unknown-pragmas -fPIC -shared -lm -o
>> csvtable.so csvtable.c
>>
>> and executed the following elisp forms in the scratch buffer:
>>
>>        (setq-local mydb (sqlite-open))
>>        (sqlite-load-extension mydb "./csvtable.so")
>>
>> I get a nil return value from the second expression, indicating
>> that it did not load the extension (verified by using the `csv` module
>> in a `sqlite-execute` call). If I try the same from the `sqlite3` cli
>> interface, it works:
>>
>>        .load ./csvtable.so
> I think you made one more change to csv.c: you renamed the function
> sqlite3_csv_init to the name sqlite3_csvtable_init.  Otherwise, the
> loading would fail, because sqlite3's cli will not find the entry
> function it expects.
>
> More importantly: the csv.c source file to which you point, viz.:
>
>    https://www.sqlite.org/src/artifact?ci=trunk&filename=ext/misc/csv.c
>
> is NOT the source file of the libsqlite3_mod_csvtable.so extension
> distributed by Debian, which we currently have on the "allow list", it
> is a different extension.  The source of csvtable is here:
>
>    https://packages.debian.org/sid/libsqlite3-mod-csvtable

bug#63590: 29.0.90; can't load sqlite extension

[Eli Zaretskii]

> Date: Sat, 20 May 2023 12:39:37 +0200
> From: Lennart Vogelsang <lennart@vogelsang.berlin>
> Cc: 63590@debbugs.gnu.org
> 
> Ahh, I just wanted to answer you, I just noticed that about the tests 
> too. Thank you! Your patch works for me,

Thanks for testing, I will install the changes soon.

> just one small thing: sqlite extension loading can also fail because of 
> other reasons (e.g. if the shared library
> does not exist). Currently your patch would leave sqlite extension 
> loading enabled in that case, I think?

Right, fixed.

> I would also argue that it would make sense to actually report the error 
> of the extension loading (when  the dynamic library file does not exist, 
> or the extension is invalid). Maybe something like this:

I don't want to change the API in such a drastic way, especially as we
are close to releasing Emacs 29.1.  It is easy for the caller to test
whether the file exists or not, and it isn't clear to me that every
caller would like the primitive to signal an error.  The return value
is enough to know whether the extension can be used or not, so every
caller can do what they need, and no important information is lost.

> That way, the test also correctly fails as we signal the error from the 
> extension loading.

I've modified the test to check the existence of the file instead.
Doing that is actually better, since on many systems the extensions
will not be installed, or installed not in the directories whose names
are used by the test, and it is important to make sure the return
value of sqlite-load-extension corresponds to whether the file is in
that place or not.

> Just out of curiosity, as there are a handful of useful 
> sqlite extensions out there, could there be a way to make the allow list 
> a bit more lenient?

I had the same thoughts, so I added some of them, which I thought
could be useful for Emacs.

> Maybe as a build configure feature allowing us to specify other
> extensions that are allowed to be loaded.

That is less desirable, since in many cases the person who builds
Emacs is some downstream distro maintainer, who doesn't know which
extensions will be useful for the end-users.

bug#63590: 29.0.90; can't load sqlite extension

[Eli Zaretskii]

> Cc: 63590@debbugs.gnu.org
> Date: Sat, 20 May 2023 15:35:17 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Date: Sat, 20 May 2023 12:39:37 +0200
> > From: Lennart Vogelsang <lennart@vogelsang.berlin>
> > Cc: 63590@debbugs.gnu.org
> > 
> > Ahh, I just wanted to answer you, I just noticed that about the tests 
> > too. Thank you! Your patch works for me,
> 
> Thanks for testing, I will install the changes soon.

Now done, and closing the bug.