orthanc/sqlitewrapper.patch

# HG changeset patch
# User Sebastien Jodogne <s.jodogne@gmail.com>
# 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);
+    }
   }