Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

SQLite: Is there a way to load extensions? #1460

Closed
d4h0 opened this issue Sep 28, 2021 · 12 comments
Closed

SQLite: Is there a way to load extensions? #1460

d4h0 opened this issue Sep 28, 2021 · 12 comments
Labels
db:sqlite Related to SQLite enhancement New feature or request proposal

Comments

@d4h0
Copy link

d4h0 commented Sep 28, 2021

Hi,

I'm looking for a way to load custom extensions into SQLite, but I can't find anything.

I'm not very experienced with SQLite and completely new to SQLx – is this somehow possible?

@abonander abonander added db:sqlite Related to SQLite proposal enhancement New feature or request labels Sep 29, 2021
@abonander
Copy link
Collaborator

TL;DR: this isn't currently supported.

There's SELECT load_extension("<file path>") but that's not enabled by default and the docs seem to highly discourage its use: https://www.sqlite.org/lang_corefunc.html#load_extension

We would probably want to expose the C API to do this, sqlite3_load_extension().

rustqlite supports this in kind of an interesting way, it provides Connection::load_extension() which is the obvious way to do it, but then provides an RAII guard to basically limit the ability to load extensions to a single scope (although you can also just manually call Connection::load_extension_enable() and ::load_extension_disable() whenever you want).

I think I would prefer to limit this even further, and only allow declaring what extensions to load in SqliteConnectOptions. This would also be the best way to load extensions when using sqlx::Pool, I think.

impl SqliteConnectOptions {
    pub fn load_extension(self, path: impl AsRef<Path>, entrypoint: Option<String>) -> SqliteConnectOptions { ... }
}

The other question is how do we make this work with the macros? If we add extensions to SqliteConnectOptions, we could support it as an option in database URLs:

sqlite://mydb.sqlite?extension=%20%path%20%to%20%extension.so
// specify a different entrypoint?
sqlite://mydb.sqlite?extension[foo]=%20%path%20%to%20%extension.so

The other option would be something like we were discussing in #121.

@d4h0
Copy link
Author

d4h0 commented Sep 29, 2021

@abonander: Thanks for your response!

There's SELECT load_extension("") but that's not enabled by default and the docs seem to highly discourage its use

The main concern is SQL injection (giving attackers access to the extension-loading functionality). So if a user is careful to deactivate extension-loading after loading all extensions, there shouldn't be a problem.

On the other hand, adding native support to SQLx for loading extensions (as you propose) shouldn't be significantly more work.

Unfortunately, I don't know C, so I can't help implement it, I believe.

The other question is how do we make this work with the macros? If we add extensions to SqliteConnectOptions, we could support it as an option in database URLs

Personally, I'd prefer a second environment variable like SQLX_SQLITE_EXTENSIONS="path/to/ext1.so:path/to/ext2.so". This would eliminate the need to URL-encode the paths.

An alternative would be to use the URL fragment for configuration, which only requires to escape #:

sqlite://mydb.sqlite#extension=/path/to/extension.so
// specify a different entrypoint?
sqlite://mydb.sqlite#extension[foo]=/path/to/extension.so

The URL fragment isn't sent from clients to servers, so this also might serve as a hint, that this is client-side configuration.

I think, SQLite extension-loading would be a great addition to SQLx. My own use-case is to load an extension for transparent row-level text compression (my project will have a huge amount of text).

I guess, for now, I have to use rusqlite and implement my own async layer (unfortunately, losing the fantastic compile-time SQL validation... 🙁)

@d4h0
Copy link
Author

d4h0 commented Oct 12, 2021

Today I thought about this a bit more...

Wouldn't it be a security risk to load database extensions via environment variables?

If an attacker can somehow change environment variables, and can add/change a database extension, he/she can execute arbitrary code, I believe.

For example, if a web application has an unsafe file upload feature, an attacker can:

  1. Upload the database extension: This is possible if the file upload functionality doesn't limit what can be uploaded, or only checks the file extension (instead of validating the file content by MIME-type sniffing). I believe, this is a pretty common situation.
  2. Change the .env file to add the database extension. This is possible if the upload feature lets clients define the name of the uploaded file on the local file system. If an attacker names the file "../../.env", he/she might be able to overwrite the `.env' file of the application (this attack is called 'path traversal attack', and it's pretty hard to escape a user supplied file name to make it safe. Therefore, it's best to not let users define the file system name of uploaded files).
  3. Restart the app: The last step would be so somehow restart the web application. The obvious way would be to somehow crash the application (which shouldn't be too difficult, most of the time).

I only recently started to study web application security, so I'm not remotely an expert in this kind of stuff. But if I can come up with a potential way to exploit such a feature, then criminal hacker surely can as well.

The above is mostly a problem if we use the environment variable to automatically load database extensions at runtime.

The benefit of this would be, that a user only needs to define database extensions once (via the DATABASE_URL variable), instead of twice (DATABASE_URL + runtime code that uses a method of the connection/pool object to load database extensions).

One easy solution would be to only use the environment variable data for the validation the macros do (as mentioned above). There would be a risk that a user extracts the database extension information from DATABASE_URL to load it via the API (because he/she wants to avoid defining the database extensions twice).

Another way would be to have some kind of global configuration macro:

sqlx::sqlite::config_database_extensions!(config);

...but I'm not sure, if that is even possible (I've never used procedural macros before).

I think it's pretty imported to think about this carefully, to not introduce a remote code execution vulnerability.

I think a "global config macro" approach would be safest, because in this case an attacker needs to be able to change the source code (which is much less likely than being able to change environment variables. The source code can't be changed at runtime, but environment variables can).

Besides the production server, other potential targets could be the machine of a developer, and the CI server (which often contains API key and other credentials). I can't come up with a potential attack scenario for these targets, but that doesn't mean that criminal hackers can't. This is another reason why I believe a global config macro would be safest.

@jplatte
Copy link
Contributor

jplatte commented Oct 12, 2021

In your attack scenario, if an attacker is able to overwrite the .env file of the web application, why would they not be able to also just overwrite the application binary already?

@dragonnn
Copy link
Contributor

environment variables doesn't mean then need to come from .env.
Also overwriting binary to keep it intact and working is harder then just editing .env

@jplatte
Copy link
Contributor

jplatte commented Oct 12, 2021

Sure, my point is just that an upload feature that's broken so severely would allow lots of other attacks too. Here's some other examples:

  • If running with sufficient privileges, you could overwrite system configuration files
  • If the user the app runs as has a working shell configured, you could likely place authorized_keys into its $HOME/.ssh directory and gain remote code execution that way if the machine has an SSH daemon running

If an attacker had control over the environment variables for a web app outside of .env, they could even just set LD_PRELOAD to load malicious code.

@dragonnn
Copy link
Contributor

I think with env variables depending how they are passed (not from .env) this could be done without write access to the host.
For example when using systemd service for passing the env variables, it doesn't load them every time it restarts the service, only at/when called systemctl daemon-reload, so if you somehow find a way to exploit systemd process and alter the running process you could change env variables for you sqlx service without writing anything to the disk.

@d4h0
Copy link
Author

d4h0 commented Oct 12, 2021

@jplatte: Sure there are several possible ways such a vulnerability can be exploited, and all have advantages and disadvantages.

For example, what will you do, if the binary hasn't a common name like main or run, etc.?

You can brute-force a name like "my-xy-application", but this is much harder than overwriting a .env file (which most-likely can be done in less than 10 requests). Depending on the target system, you might even run out of disk space before you find the right name.

Also, if you replace the main binary, it's immediately obvious that something is wrong (because the web application doesn't work anymore, after you trigger the restart). An infected database probably can go undetected for a long time.

And you only have one attempt. If you make any mistake, you get caught (or at least can't try again because th web application is down). A database extension can be tested locally to be relatively confident, that it won't crash the database and web application.

Besides that, my attack scenario was just an example I came up (in a few seconds) to illustrate how such a feature might be exploited.

I don't know enough about Linux (let alone other operating systems) to be confident that there aren't other ways to exploit this feature (if the environment variable is used at runtime). You never know in what environment someone is running something and what other vulnerabilities exist, etc.

Maybe every possible way to exploit this feature would also enable another attack with similar or higher severity, maybe this feature would enable unique exploits, who knows?

So if there is a way to implement something in a way that might be exploitable, and there is a way to implement it in a way that can't be exploited (without having write access to the source code) — why would we choose the former?

I don't believe, that preventing the need to duplicate one line of code is worth it, to be honest.

Remote code execution is generally a vulnerability of the highest severity, so I'd be rather safe than sorry – especially with a library that is used as much as sqlx is used.

@abonander
Copy link
Collaborator

I'm going to make a judgement call here, and say that to obviate various issues, the list of extensions to load should more often than not be hardcoded anyway so I now think it should not be part of the database URL or determined by an environment variable.

This makes it harder to forget to specify extensions to be loaded at runtime that are important for an application's function, such as ones that add new SQL functions. You can parse a SqliteConnectOptions from the DATABASE_URL and then add extensions to it:

let mut options = SqliteConnectOptions::parse(&env::var("DATABASE_URL")?)?;

options.load_extension("sqlite-std");

let pool = SqlitePool::connect_with(options).await?;

As for the macros, it's the same thing. You don't want to forget to list extensions in the database URL or miscellaneous environment variable that are necessary to compile, so it makes sense to specify them in the application code:

sqlx::config_macros!(sqlite_extension = "sqlite-zstd");

If you want the list of extensions to be runtime-specifiable still, you can integrate that into your application:

for ext in env::var("SQLITE_EXTENSIONS").ok().unwrap_or_else(String::new).split(",") {
    options.load_extension(ext);
}

@abonander
Copy link
Collaborator

closed by #2062

@skelewir
Copy link

skelewir commented Jan 19, 2023

So, is there any way to load extensions for macros?
And is there a way to specify some connection parameters to use for macro connection?
Same for migrations.

@mattfbacon
Copy link
Contributor

Still can't specify extensions for sqlx migrate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
db:sqlite Related to SQLite enhancement New feature or request proposal
Projects
None yet
Development

No branches or pull requests

6 participants