Skip to content

Load SQLite extensions via SQLite C-API #319

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Load SQLite extensions via SQLite C-API #319

wants to merge 7 commits into from

Conversation

Andy-2639
Copy link
Contributor

This allows to load SQLite extensions without enabling the SQL function load_extension.

@bradh
Copy link

bradh commented May 23, 2019

Is there a way forward on this PR?

@StevenLooman
Copy link

I am interested in this as well. What is the way forward?

@alun
Copy link

alun commented May 23, 2020

I found it's relatively easy to do without this change:

  1. Enable load extension in the connection properties like here
    prop.setProperty("enable_load_extension", "true");

Also possible in DBeaver:
Screenshot 2020-05-23 at 16 12 20

  1. Use SQL to load the extension e.g. SELECT load_extension('/path/to/extension/md5', 'sqlite3_extension_init');

Note the shared library extension must be omitted.

P. S. I've compiled md5 from this source https://github.com/moisseev/sqlite-md5/blob/master/md5.c on macOS using clang CLI:

cc -c md5.c
cc -dynamiclib md5.o -o md5.dylib

@Andy-2639
Copy link
Contributor Author

I found it's relatively easy to do without this change:

Yes, it is. It just allows extensions - arbitrary code - to be loaded into your process by normal SQL queries or SQL injection attacks.
I mean, you have to take care of SQL injections anyway (prepared statements, ...) but humans make errors and enabling enable_load_extension increases the ability of an attacker in case of an possible injection attack IMO.

@alun
Copy link

alun commented May 23, 2020

@Andy-2639 cool, I see you point. I'd say those parts of a program calling load_extension stuff shouldn't be exposed to a non-authorised user's input data. And adding more native calls perhaps enlarges the attacking surface by itself.
What about this function here

public native synchronized int enable_load_extension(boolean enable);

can it be used to disable the feature once all required extensions are loaded? Although it might be added after your PR.

@Andy-2639
Copy link
Contributor Author

Andy-2639 commented May 25, 2020

@alun Yeah,

public native synchronized int enable_load_extension(boolean enable);

should work. However, I don't see it exposed.

So I guess when

public abstract class SQLiteConnection

exposes enable_load_extension and
public Connection createConnection(String url) throws SQLException {

would return a SQLiteConnection, the following would be possible:

  1. Create SQLite connection
  2. Enable extension loading
  3. Load extension
  4. Disable extension loading
  5. Run queries with user provided data (still properly escaping/quoting it - prepared statements(!) - to protect from sql injections)

Edit: Changing the return type is not an option because it breaks API compatibility: https://wiki.eclipse.org/Evolving_Java-based_APIs_2#Evolving_API_interfaces_-_API_methods

@Andy-2639
Copy link
Contributor Author

I sketched a possible solution to disable extension loading after loading them with the enable_load_extension method: #317 (comment)

@gotson gotson added impacts:JNI Has impact on JNI C code enhancement:SQLite Enhancement about sqlite features labels Jul 29, 2022
@gotson
Copy link
Collaborator

gotson commented Jul 29, 2022

@Andy-2639 given this is 4 years old, it needs to be updated to the latest version. If you can do that i can review the code, i think it would be a good addition to this project.

@gotson gotson added the waiting for feedback Waiting for a feedback from the issue creator label Jul 29, 2022
@Andy-2639
Copy link
Contributor Author

Sorry @gotson, as this PR needs the native libs rebuilt and I currently have no Linux system available to do this, caring about this PR takes too much time for me.

I'm fine with the workaround described in #317 (comment) despite the nasty downcast.
In this PR, I really don't like the serialization I needed to store the extensions to load in the properties ...

@gotson
Copy link
Collaborator

gotson commented Aug 1, 2022

this PR needs the native libs rebuilt and I currently have no Linux system available to do this,

Almost everything is built via Dockcross now, should work well on WSL on Windows too.


I will mark this as needing rework, if someone wants to pickup your work

@gotson gotson added needs rework and removed waiting for feedback Waiting for a feedback from the issue creator labels Aug 1, 2022
@gotson gotson marked this pull request as draft August 1, 2022 03:56
@gotson gotson removed the request for review from xerial August 1, 2022 03:56
@gotson gotson added the help wanted Contributions are welcome label Aug 1, 2022
Repository owner deleted a comment from PortDm Feb 17, 2025
Repository owner deleted a comment from PortDm Feb 17, 2025
@magicprinc

This comment was marked as off-topic.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement:SQLite Enhancement about sqlite features help wanted Contributions are welcome impacts:JNI Has impact on JNI C code needs rework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants