changeset 3809:2bf30ef727e3

enforcing SQLiteDatabaseWrapper::GetTableRecordCount() against SQL injection
author Sebastien Jodogne <s.jodogne@gmail.com>
date Fri, 03 Apr 2020 14:47:37 +0200
parents 7f083dfae62b
children e9b7e05bcd42 c81ac6ff232b
files OrthancServer/Database/SQLiteDatabaseWrapper.cpp
diffstat 1 files changed, 26 insertions(+), 9 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancServer/Database/SQLiteDatabaseWrapper.cpp	Fri Apr 03 14:06:13 2020 +0200
+++ b/OrthancServer/Database/SQLiteDatabaseWrapper.cpp	Fri Apr 03 14:47:37 2020 +0200
@@ -295,19 +295,36 @@
 
   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);
+    /**
+     * "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++)
+    {
+      if (!isalpha(table[i]))
+      {
+        throw OrthancException(ErrorCode_ParameterOutOfRange);
+      }
+    }
 
-    if (!s.Step())
+    // Don't use "SQLITE_FROM_HERE", otherwise "table" would be cached
+    SQLite::Statement s(db_, "SELECT COUNT(*) FROM " + table);
+
+    if (s.Step())
+    {
+      int64_t c = s.ColumnInt(0);
+      assert(!s.Step());
+      return c;
+    }
+    else
     {
       throw OrthancException(ErrorCode_InternalError);
     }
-
-    int64_t c = s.ColumnInt(0);
-    assert(!s.Step());
-
-    return c;
   }