Crash in zipfile(NULL) in 3.40.0
(1) By PaulK (paulclinger) on 2023-01-04 04:46:48 [source]
I get a crash with SIGSEGV on
select * from zipfile(NULL) with the following stack trace:
70000003dbe0 5778c8 fopen+40 70000003f890 4933ed zipfileFilter+173 70000003f8d0 470fbb sqlite3VdbeExec+20690 70000003f9f0 465b10 sqlite3_step+320
This is somewhat similar to an earlier report about a crash in
zipfile(zeroblob('test.zip')) (https://sqlite.org/forum/forumpost/ae86934905929b96fb5fd1cb947eaaa9a680580541359ab132e8901bc0b52db5). I'm not going to pass NULL normally, but wouldn't expect a crash here either (but rather "Runtime error: cannot open file:").
(2) By Larry Brasfield (larrybr) on 2023-01-04 08:37:03 in reply to 1 [link] [source]
Could you please run:
and report what is output?
I would not expect a crash either, and would like to find out why you saw that, either to fix it or be sure it was fixed later. I do not see that with current source.
(3) By PaulK (paulclinger) on 2023-01-05 01:06:04 in reply to 2 [link] [source]
SQLite version 3.40.0 2022-11-16 12:10:08 Enter ".help" for usage hints. Connected to a ←[1mtransient in-memory database←[0m. Use ".open FILENAME" to reopen on a persistent database. sqlite>: select sqlite_source_id(); 2022-11-16 12:10:08 89c459e766ea7e9165d0beeb124708b955a4950d0f4792f457465d71b158d318
I do not see that with current source.
Interesting. Thank you for checking on this. This is compiled with a special library, but it does fail for me in SQLite CLI as well with no error message.
What result do you get when you run it?
(4) By Larry Brasfield (larrybr) on 2023-01-05 01:13:38 in reply to 3 [link] [source]
What result do you get when you run it?
Assuming "it" is the same version you showed, I get this:
SQLite version 3.40.0 2022-11-16 12:10:08
Enter ".help" for usage hints.
Connected to a transient in-memory database.
Use ".open FILENAME" to reopen on a persistent database.
sqlite> select * from zipfile(NULL);
Runtime error: cannot open file:
, with the Linux i386 build on a recent Xubuntu system, 5.15.0-56-generic kernel.
(6) By Richard Hipp (drh) on 2023-01-05 01:42:13 in reply to 4 [link] [source]
The problem shows up for me if you run "sqlite3" under valgrind. Otherwise, I just get the error like Larry.
I checked in a fix. I also cherry-picked the fix onto branch-3.40 and I increased the version number for builds on that branch to 3.40.2. That does not mean that there will be a 3.40.2 release, however. The problem was a NULL pointer passed into the filename argument of the fopen() standard library function. In most implementations of the standard library, that seems to be benign. The fopen() fails and life goes on. And even if it does cause mischief, an attacker would have to already have the ability to read and write arbitrary files on the filesystem (because they can invoke zipfile()) in order to carry out the attack, so it not clear that triggering a NULL pointer dereference in fopen() gives them any additional power.
Note also that the bug is in the zipfile extension, which is not part of the SQLite amalgamation, though it is included in the CLI.
So I don't think this is a high-severity bug. But it is fixed on trunk and on branch-3.40, and the fix will be in the next release, whatever that release is called.
(8) By PaulK (paulclinger) on 2023-01-05 03:58:51 in reply to 6 [link] [source]
Thank you, Richard, for the quick fix! Your explanation matches the behavior I observe (and I do run this as part of the cosmopolitan project that implements its own libc, so it may behave differently in its fopen implementation).
(9) By PaulK (paulclinger) on 2023-01-08 23:10:21 in reply to 6 [link] [source]
Richard, would this also fix the similar result from doing
SELECT name, data FROM zipfile(cast('' as blob));, which also produces SIGSEGV? I'm not able to test the fix in my environment at the moment unfortunately.
(10) By Larry Brasfield (larrybr) on 2023-01-09 03:06:31 in reply to 9 [link] [source]
The fix detects the (C-)NULL filename pointer from whatever form of argument given to zipfile() can have that value. This includes your cast('' as blob) case.
Short answer: Yes.
(5) By Larry Brasfield (larrybr) on 2023-01-05 01:38:12 in reply to 3 [link] [source]
In a check-in just made, Richard has eliminated what appears to be an obvious bug enabling the misbehavior you reported. I'm not sure (and mystified as to) why I do not see the 3.40.0 release misbehaving. But it's fixed now on trunk and in whatever CLI is next released.
(7) By PaulK (paulclinger) on 2023-01-05 03:55:37 in reply to 5 [link] [source]
In most implementations of the standard library, that seems to be benign.
I run it as part of a project that implements its own libc, which appears to be causing the difference in the behavior.