postiffm / bibledit-desktop

Desktop version of Bibledit
GNU General Public License v3.0
4 stars 6 forks source link

Wrongly using SQLite APIs can cause information leakage and DoS #133

Open Zero0one1 opened 4 years ago

Zero0one1 commented 4 years ago

bug report

missing sqlite3_close() after sqlite3_open()

Hi, developers:

There may be several bugs due to calling sqlite3_open(filename, db) while missing call sqlite3_close(db) after use. And I've reported at bibledit-desktop-old

According to SQLite Documention :

Whether or not an error occurs when it is opened, resources associated with the database connection handle should be released by passing it to sqlite3_close() when it is no longer required.

Since the connection number of database and resource are limited, if we don't close connection in time, it would leak information and affect process running.

Specific source code locations are below:

src/stylesheetutils.cpp:400

395 void stylesheet_set_recently_used(const ustring & stylesheet, vector <ustring>& styles, vector <unsigned int>& counts)
396 {
397   // Connect to db.
398   ustring filename = stylesheet_recent_filename();
399   sqlite3 *db;
400   sqlite3_open(filename.c_str(), &db);
401   char *sql;

src/stylesheetutils.cpp:380

371 void stylesheet_get_recently_used(const ustring & stylesheet, vector <ustring>& markers, vector <unsigned int>& count)
372 // Read the recently used data: markers and usage count.
373 { 
...
379   sqlite3 *db;
380   sqlite3_open(stylesheet_recent_filename().c_str(), &db);
381   sqlite3_busy_timeout(db, 1000);

src/statistics.cpp:95 Before throw runtime_error in line 105, 112, 130, it doesn’t close the db.

 64 void statistics_initial_check_project(const ustring & project, bool gui)
...
 92     sqlite3 *db;
 93     int rc;
 94     try {
 95       rc = sqlite3_open(filename.c_str(), &db);
 96       if (rc)
 97         throw runtime_error(sqlite3_errmsg(db));
 98       char *sql;

missing sqlite3_free() after sqlite3_exec()

There are maybe several bugs due to calling sqlite3_exec(db, sql, callback, NULL, errmsg) while missing call sqlite3_free(errmsg) after use.

SQLite Documention mentions:

If the 5th parameter to sqlite3_exec() is not NULL then any error message is written into memory obtained from sqlite3_malloc() and passed back through the 5th parameter. To avoid memory leaks, the application should invoke sqlite3_free()

Otherwise, the memory consumption might be huge, which may become a DoS.

several examples are below:

/src/keyterms.cpp:131

131    rc = sqlite3_exec(db, sql, NULL, NULL, &error);

There is the whole list of locations related to this problem:

src/keyterms.cpp:131
src/keyterms.cpp:139
src/keyterms.cpp:163
src/keyterms.cpp:218
src/keyterms.cpp:226
src/keyterms.cpp:328
src/keyterms.cpp:336
src/keyterms.cpp:426
src/keyterms.cpp:434
src/keyterms.cpp:694
src/keyterms.cpp:702
src/keyterms.cpp:807
src/keyterms.cpp:904
src/keyterms.cpp:926
src/keyterms.cpp:956
src/keyterms.cpp:987
src/keyterms.cpp:997
src/keyterms.cpp:1006
src/keyterms.cpp:1040
src/keyterms.cpp:1067
src/keyterms.cpp:1095
src/keyterms.cpp:1104
src/keyterms.cpp:1187
src/keyterms.cpp:1209
src/keyterms.cpp:1228
src/kjv.cpp:311
src/kjv.cpp:275