1315192 - [Static Analysis][Out-of-bounds read] function sqlite3TreeViewLine from db/sqlite3/src/sqlite3.c

Status: VERIFIED INVALID

(Core :: SQLite and Embedded Database Bindings, defect)

Description

[Cervantes Yu [:cyu] [:cervantes]]

Coverity reported that in function sqlite3TreeViewLine(), the following line:
> if( zBuf[acc.nChar-1]!='\n' ) sqlite3StrAccumAppend(&acc, "\n", 1);
could have out-of-bounds read if acc.nChar is 0. This could happen if argument p (of type TreeView*) is null, which could happen when in
> sqlite3TreeViewSelect(0, p, 0);
calls when build flag SELECTTRACE_ENABLED is turned on.

Comment 1

[D. Richard Hipp]

(1) The sqlite3TreeViewLine() method (and the entire TreeView object) are contained within #ifdef SQLITE_DEBUG....#endif.  This code is for analysis and debugging only and is not normally included in production builds.  FF is not being compiled with SQLITE_DEBUG is it?  Note that compiling with SQLITE_DEBUG makes SQLite approximately 3 times slower!

(2) Coverity is incorrect.  acc.nChar is always greater than zero due to the prior call to sqlite3VXPrintf() in which the zFormat parameter is always a non-empty string.

(3) An assert() is added by https://www.sqlite.org/src/info/97354093bceff287 that will likely silence this coverity false-positive.  That change will appear in the 3.16.0 release.  That change will not be in the 3.15.1 release because it is not a bug fix.

Comment 2

[Cervantes Yu [:cyu] [:cervantes]]

Oh, I didn't notice that the nChar is always >0 after the sqlite3VXPrintf() call.
SQLITE_DEBUG is not turned on in building FF. Thanks for the assertion to make this clear.

Comment 3

[Marco Bonardo [:mak]]

(In reply to D. Richard Hipp from comment #1)
> (1) The sqlite3TreeViewLine() method (and the entire TreeView object) are
> contained within #ifdef SQLITE_DEBUG....#endif.  This code is for analysis
> and debugging only and is not normally included in production builds.  FF is
> not being compiled with SQLITE_DEBUG is it?  Note that compiling with
> SQLITE_DEBUG makes SQLite approximately 3 times slower!

nope, we don't use SQLITE_DEBUG

Thank you for the quick reply!