Fix GH-9032: SQLite3 authorizer crashes on NULL values #9040
Conversation
The arguments 3 to 6 of the authorizer callback may be `NULL`[1], and we have to properly deal with that. Instead of causing a segfault, we deny authorization, which is still better than a crash, and apparently, we cannot do better anyway. [1] <https://www.sqlite.org/c3ref/set_authorizer.html>
|
|
||
| try { | ||
| $st = $db->prepare('attach database :a AS "db2"'); | ||
| $st->bindValue("a", ":memory:"); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
No, I'm afraid that is just not possible. You would need to dynamically construct the SQL query instead.
There was a problem hiding this comment.
👍 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
There was a problem hiding this comment.
Ah, right, for ext/sqlite3 you could overwrite the authorizer callback; that is not (yet) supported for ext/pdo_sqlite, though.
| @@ -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); | |||
There was a problem hiding this comment.
glad you thought of it, kind of disturbed me when I saw it the other day 👍
|
Is there any possibility I can get GH access to submit reviews and possibly update issues (edit, labels, ...)? |
|
If there are no objections, I'll merge this by the end of the week. |
The arguments 3 to 6 of the authorizer callback may be
NULL[1], andwe have to properly deal with that. Instead of causing a segfault, we
deny authorization, which is still better than a crash, and apparently,
we cannot do better anyway.
[1] https://www.sqlite.org/c3ref/set_authorizer.html
Note that
SQLITE_COPYis unused as of SQLite 3.6.10 (or earlier), but even for PHP-8.0 we require SQLite > 3.7.4, so there is no need to add that tophp_sqlite3_authorizer(). I'll file a PR to remove that clause from master.