# HG changeset patch # User Sebastien Jodogne # Date 1585918057 -7200 # Node ID 2bf30ef727e3c0c0498be75eecb253efbb3c1070 # Parent 7f083dfae62b7031fd964681e7ab01e576e49d54 # enforcing SQLiteDatabaseWrapper::GetTableRecordCount() against SQL injection diff -r 7f083dfae62b -r 2bf30ef727e3 OrthancServer/Database/SQLiteDatabaseWrapper.cpp --- OrthancServer/Database/SQLiteDatabaseWrapper.cpp Fri Apr 03 14:06:13 2020 +0200 +++ OrthancServer/Database/SQLiteDatabaseWrapper.cpp Fri Apr 03 14:47:37 2020 +0200 @@ -296,17 +296,34 @@ int64_t SQLiteDatabaseWrapper::GetTableRecordCount(const std::string& table) { - char buf[128]; - sprintf(buf, "SELECT COUNT(*) FROM %s", table.c_str()); - SQLite::Statement s(db_, buf); - - if (!s.Step()) + /** + * "Generally one cannot use SQL parameters/placeholders for + * database identifiers (tables, columns, views, schemas, etc.) or + * database functions (e.g., CURRENT_DATE), but instead only for + * binding literal values." => To avoid any SQL injection, we + * check that the "table" parameter has only alphabetic + * characters. + * https://stackoverflow.com/a/1274764/881731 + **/ + for (size_t i = 0; i < table.size(); i++) { - throw OrthancException(ErrorCode_InternalError); + if (!isalpha(table[i])) + { + throw OrthancException(ErrorCode_ParameterOutOfRange); + } } - int64_t c = s.ColumnInt(0); - assert(!s.Step()); + // Don't use "SQLITE_FROM_HERE", otherwise "table" would be cached + SQLite::Statement s(db_, "SELECT COUNT(*) FROM " + table); - return c; + if (s.Step()) + { + int64_t c = s.ColumnInt(0); + assert(!s.Step()); + return c; + } + else + { + throw OrthancException(ErrorCode_InternalError); + } }