Closed Bug 1315192 Opened 3 years ago Closed 3 years ago

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

Categories

(Toolkit :: Storage, defect)

defect
Not set

Tracking

()

VERIFIED INVALID
Tracking Status
firefox52 --- affected

People

(Reporter: cyu, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: CID 1368316)

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.
Whiteboard: CID 1368316
(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.
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.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INVALID
(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!
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.