Skip to content

Fix GH-9032: SQLite3 authorizer crashes on NULL values #9040

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Fix GH-9032: SQLite3 authorizer crashes on NULL values #9040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions ext/pdo_sqlite/sqlite_driver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,9 @@ static const struct pdo_dbh_methods sqlite_methods = {

static char *make_filename_safe(const char *filename)
{
if (!filename) {
return NULL;
}
if (*filename && memcmp(filename, ":memory:", sizeof(":memory:"))) {
char *fullpath = expand_filepath(filename, NULL);

Expand All @@ -737,7 +740,7 @@ static int authorizer(void *autharg, int access_type, const char *arg3, const ch
char *filename;
switch (access_type) {
case SQLITE_COPY: {
filename = make_filename_safe(arg4);
filename = make_filename_safe(arg4);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glad you thought of it, kind of disturbed me when I saw it the other day 👍

if (!filename) {
return SQLITE_DENY;
}
Expand All @@ -746,7 +749,7 @@ static int authorizer(void *autharg, int access_type, const char *arg3, const ch
}

case SQLITE_ATTACH: {
filename = make_filename_safe(arg3);
filename = make_filename_safe(arg3);
if (!filename) {
return SQLITE_DENY;
}
Expand Down
26 changes: 26 additions & 0 deletions ext/pdo_sqlite/tests/gh9032.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
--TEST--
SQLite3 authorizer crashes on NULL values
--SKIPIF--
<?php
if (!extension_loaded("pdo_sqlite")) die("skip pdo_sqlite extension not available");
?>
--INI--
open_basedir=.
--FILE--
<?php
$db = new PDO("sqlite::memory:", null, null, [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION]);

$db->exec('attach database \':memory:\' AS "db1"');
var_dump($db->exec('create table db1.r (id int)'));

try {
$st = $db->prepare('attach database :a AS "db2"');
$st->execute([':a' => ':memory:']);
var_dump($db->exec('create table db2.r (id int)'));
} catch (PDOException $ex) {
echo $ex->getMessage(), PHP_EOL;
}
?>
--EXPECT--
int(0)
SQLSTATE[HY000]: General error: 23 not authorized
3 changes: 3 additions & 0 deletions ext/sqlite3/sqlite3.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2049,6 +2049,9 @@ static int php_sqlite3_authorizer(void *autharg, int action, const char *arg1, c
/* Check open_basedir restrictions first */
if (PG(open_basedir) && *PG(open_basedir)) {
if (action == SQLITE_ATTACH) {
if (!arg1) {
return SQLITE_DENY;
}
if (memcmp(arg1, ":memory:", sizeof(":memory:")) && *arg1) {
if (strncmp(arg1, "file:", 5) == 0) {
/* starts with "file:" */
Expand Down
28 changes: 28 additions & 0 deletions ext/sqlite3/tests/gh9032.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
--TEST--
SQLite3 authorizer crashes on NULL values
--SKIPIF--
<?php
if (!extension_loaded("sqlite3")) die("skip sqlite3 extension not available");
?>
--INI--
open_basedir=.
--FILE--
<?php
$db = new SQLite3(":memory:");
$db->enableExceptions(true);

$db->exec('attach database \':memory:\' AS "db1"');
var_dump($db->exec('create table db1.r (id int)'));

try {
$st = $db->prepare('attach database :a AS "db2"');
$st->bindValue("a", ":memory:");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I discovered this issue by using a database abstraction layer which bind strings as params. Is there any easy way to support the ATTACH DATABASE with value passed by param together with open_basedir?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I'm afraid that is just not possible. You would need to dynamically construct the SQL query instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I see, it is also documented in https://www.php.net/manual/en/sqlite3.setauthorizer.php and it seems the only supported stage by Sqlite http://sqlite.org/c3ref/set_authorizer.html

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right, for ext/sqlite3 you could overwrite the authorizer callback; that is not (yet) supported for ext/pdo_sqlite, though.

$st->execute();
var_dump($db->exec('create table db2.r (id int)'));
} catch (Exception $ex) {
echo $ex->getMessage(), PHP_EOL;
}
?>
--EXPECT--
bool(true)
Unable to prepare statement: 23, not authorized