Two crashes found through fuzzing
(1) By Maik (maikbe) on 2022-04-25 09:29:45 [source]
I am currently evaluating a grammar-based fuzzer and stumbled accross two unique crashes that I don't know how to interpret. Perhaps you can help?
- sqlite3.c version:
- fuzzershell.c version:
CREATE TABLE IF NOT EXISTS t1 ( c2 ) ; CREATE TEMP TRIGGER IF NOT EXISTS trig0 BEFORE UPDATE OF c2 ON t1 BEGIN INSERT OR FAIL INTO t1 VALUES ( c2 ) ; END ; EXPLAIN CREATE TEMP TRIGGER IF NOT EXISTS trig0 BEFORE UPDATE OF c2 ON t1 BEGIN INSERT OR FAIL INTO t1 ( c2 ) VALUES ( TRUE ) RETURNING * ; END ;
... TRACE: CREATE TABLE IF NOT EXISTS t1 ( c2 ) ; TRACE: CREATE TEMP TRIGGER IF NOT EXISTS trig0 BEFORE UPDATE OF c2 ON t1 BEGIN INSERT OR FAIL INTO t1 VALUES ( c2 ) ; END ; fuzzershell: sqlite3.c:113999: sqlite3FinishCoding: Assertion `0' failed. Aborted (core dumped)
The last EXPLAIN together with the CREATE TEMP TRIGGER statement seem to cause the error. When leaving out the TRIGGER statement then the EXPLAIN statement returns the following output:
... TRACE: CREATE TABLE IF NOT EXISTS t1 ( c2 ) ; LOG: (1) cannot use RETURNING in a trigger in "EXPLAIN CREATE TEMP TRIGGER IF NOT EXISTS trig0 BEFORE UPDATE OF c2 ON t1 BEGIN INSERT OR FAIL INTO t1 ( c2 ) VALUES ( TRUE ) RETURNING * ; END ; " RESULT-CODE: 0
This is interesting because the RETURNING part should be evaluated as a part of the INSERT statement inside the BEGIN ... END part and not as a part of the EXPLAIN statement. Note that this only fails with the NEVER() assertions being turned on.
I've checked the documentation regarding the EXPLAIN statement and found that it should only be used for interactive analysis, so I guess nothing critical here.
Though, I am not sure whether this is a (parser?) bug or not.
Another crash when using TRIGGER + ALTER statements, but this time I think it is a false positive, but I am not entirely sure.
CREATE TABLE IF NOT EXISTS t1 ( pgsize ) ; CREATE TEMP TRIGGER trig1 DELETE ON t1 FOR EACH ROW BEGIN INSERT OR FAIL INTO t1 ( pgsize , pgsize ) VALUES ( CURRENT_DATE ) , ( TRUE ) ON CONFLICT ( RAISE ( IGNORE ) COLLATE BINARY ) DO NOTHING ; VALUES ( NULL , RAISE ( ABORT , 'ahh' ) ) UNION ALL SELECT ALL * , * FROM main . t1 t1 GROUP BY t1 . pgsize IS main . t1 . pgsize , RAISE ( FAIL , 'ahh' ) WINDOW win2 AS ( win0 PARTITION BY CURRENT_TIMESTAMP , t1 . pgsize IS RAISE ( IGNORE ) ) ORDER BY t1 . pgsize DESC , RAISE ( IGNORE ) COLLATE BINARY ; END ; ALTER TABLE t1 RENAME pgsize TO pgsize ;
... TRACE: ALTER TABLE t1 RENAME pgsize TO pgsize ; fuzzershell: sqlite3.c:109505: renameTokenCheckAll: Assertion `p->p!=pPtr' failed. Aborted (core dumped)
I know that renaming a column to its own name is non-sense. According to the documentation I assume that renaming a column to the same name is not exactly allowed. However, this particular assertion is not violated when not using the ALTER statement without the trigger, which is why I posted the crash here anyways.
Note that I found both crashes multiple times, so if you require further (even less readable) examples, don't hesitate to ask.
(2) By Dan Kennedy (dan) on 2022-04-25 11:02:28 in reply to 1 [link] [source]
Thanks for finding and reporting these.
These both cause assert failures in [fa1b393bdb66b985], but run clean on the tip of trunk (bd6811d8110d5f00). I imagine they were caught by some other fuzzer and fixed at some point.
(3.1) By Richard Hipp (drh) on 2022-04-25 11:13:50 edited from 3.0 in reply to 1 [link] [source]
Thanks for the bug reports. Both errors occur in debug builds only and are harmless for production builds.
Crash #1 is a NEVER() macro. NEVER() macros are macros we put around boolean expressions that we believe must always be false. The macro is a no-op for production builds, but raises an assertion for debug builds. You found a case that makes one of these booleans true. The code has now been corrected by check-in bd6811d8110d5f00.
Crash #2 is a failure in renameTokenCheckAll(). That routine only exists in debugging builds. Its purpose is to find cases where pointer comparisons are done on pointers to objects that have been previously freed. Such comparisons are technically "undefined behavior", but as all modern systems implement pointers as integers of the appropriate size (as well they should!) the comparisons are utterly harmless. Nevertheless, we did implement the renameTokenCheckAll() routine to find this UB using ASAN, just to be pedantic. The problem was fixed on 2022-02-12 by check-in 9252619d410293dd.
(4.1) By Maik (maikbe) on 2022-04-25 11:46:05 edited from 4.0 in reply to 3.1 [link] [source]
Thank you for the quick and detailed response! Really helped me a lot to sort these things out.
Have a nice day! :-)