Crash on failure in database corruption detection, and patch

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Crash on failure in database corruption detection, and patch

Even Rouault
Hi,

oss-fuzz on GDAL has spotted a crashing bug in SQLite:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10750 (not accessible
yet)

Here's the way to reproduce it:

$ valgrind ~/install-sqlite-3.25.1/bin/sqlite3 corrupt.db \
        "SELECT 1 FROM gpkg_tile_matrix WHERE table_name = 'my' AND \
        pixel_x_size > 0 ORDER BY zoom_level DESC"

[...]
==8944== Invalid read of size 1
==8944==    at 0x46202C: sqlite3VdbeIdxRowid (sqlite3.c:80107)
==8944==    by 0x46C934: sqlite3VdbeExec (sqlite3.c:88086)
==8944==    by 0x4634D1: sqlite3Step (sqlite3.c:81035)
==8944==    by 0x46371F: sqlite3_step (sqlite3.c:81098)
==8944==    by 0x417CD5: exec_prepared_stmt (shell.c:10255)
==8944==    by 0x4187CB: shell_exec (shell.c:10523)
==8944==    by 0x427918: main (shell.c:16263)
==8944==  Address 0x105fe5de7 is not stack'd, malloc'd or (recently) free'd
==8944==
==8944==
==8944== Process terminating with default action of signal 11 (SIGSEGV)
==8944==  Access not within mapped region at address 0x105FE5DE7
==8944==    at 0x46202C: sqlite3VdbeIdxRowid (sqlite3.c:80107)
==8944==    by 0x46C934: sqlite3VdbeExec (sqlite3.c:88086)
==8944==    by 0x4634D1: sqlite3Step (sqlite3.c:81035)
==8944==    by 0x46371F: sqlite3_step (sqlite3.c:81098)
==8944==    by 0x417CD5: exec_prepared_stmt (shell.c:10255)
==8944==    by 0x4187CB: shell_exec (shell.c:10523)
==8944==    by 0x427918: main (shell.c:16263)

I've come up with this simple patch that fixes the issue (the cast to (int) or
a large unsigned value not fitting on int is undefined behaviour in C)

$ diff -u sqlite3.c.bak sqlite3.c
--- sqlite3.c.bak 2018-09-30 18:43:28.248503877 +0200
+++ sqlite3.c 2018-09-30 18:43:38.520405935 +0200
@@ -80098,7 +80098,7 @@
   (void)getVarint32((u8*)m.z, szHdr);
   testcase( szHdr==3 );
   testcase( szHdr==m.n );
-  if( unlikely(szHdr<3 || (int)szHdr>m.n) ){
+  if( unlikely(szHdr<3 || szHdr>(unsigned)m.n) ){
     goto idx_rowid_corruption;
   }

I can reproduce this on versions at least since sqlite 3.11.0, but I see older
versions have the same faulty code, so this is probably a difference in query
planner behaviour not triggering the bug on that particular query.

Best regards,

Even

--
Spatialys - Geospatial professional services
http://www.spatialys.com
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users