Merge pull request #14647 from microsoft/24-odbc-model-instantiation-upstream2

C++: Adding a model implementation for ODBC.

Author: MathiasVP

cpp/ql/lib/semmle/code/cpp/models/Models.qll

@@ -35,6 +35,7 @@ private import implementations.Accept
 private import implementations.Poll
 private import implementations.Select
 private import implementations.MySql
+private import implementations.ODBC
 private import implementations.SqLite3
 private import implementations.PostgreSql
 private import implementations.System

cpp/ql/lib/semmle/code/cpp/models/implementations/ODBC.qll

@@ -0,0 +1,28 @@
+/**
+ * Provides implementation classes modeling the ODBC C/C++ API.
+ * See `semmle.code.cpp.models.Models` for usage information.
+ */
+
+private import semmle.code.cpp.models.interfaces.Sql
+private import semmle.code.cpp.models.interfaces.FunctionInputsAndOutputs
+
+/**
+ * The `SQLExecDirect`, and `SQLPrepare` from the ODBC C/C++ API:
+ * https://learn.microsoft.com/en-us/sql/odbc/reference/syntax/sqlexecdirect-function?view=sql-server-ver16
+ * https://learn.microsoft.com/en-us/sql/odbc/reference/syntax/sqlprepare-function?view=sql-server-ver16
+ *
+ * Note, `SQLExecute` is not included because it operates on a SQLHSTMT type, not a string.
+ * The SQLHSTMT parameter for `SQLExecute` is set through a `SQLPrepare`, which is modeled.
+ * The other source of input to a `SQLExecute` is via a `SQLBindParameter`, which sanitizes user input,
+ * and would be considered a barrier to SQL injection.
+ */
+private class ODBCExecutionFunction extends SqlExecutionFunction {
+  ODBCExecutionFunction() { this.hasGlobalName(["SQLExecDirect", "SQLPrepare"]) }
+
+  override predicate hasSqlArgument(FunctionInput input) { input.isParameterDeref(1) }
+}
+// NOTE: no need to define a barrier explicitly.
+// `SQLBindParameter` is the typical means for sanitizing user input.
+//       https://learn.microsoft.com/en-us/sql/odbc/reference/syntax/sqlbindparameter-function?view=sql-server-ver16
+// First a query is establisehed via `SQLPrepare`, then parameters are bound via `SQLBindParameter`, before
+// the query is executed via `SQLExecute`. We are not modeling SQLExecute, so we do not need to model SQLBindParameter.

cpp/ql/src/change-notes/2023-10-31-odbc-models.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added SQL API models for `ODBC`.

cpp/ql/test/query-tests/Security/CWE/CWE-089/SqlTainted/SqlTainted.expected

@@ -4,6 +4,8 @@ edges
 | test.c:35:16:35:23 | userName indirection | test.c:40:25:40:32 | username indirection |
 | test.c:38:7:38:20 | globalUsername indirection | test.c:51:18:51:23 | query1 indirection |
 | test.c:40:25:40:32 | username indirection | test.c:38:7:38:20 | globalUsername indirection |
+| test.c:75:8:75:16 | gets output argument | test.c:76:17:76:25 | userInput indirection |
+| test.c:75:8:75:16 | gets output argument | test.c:77:20:77:28 | userInput indirection |
 | test.cpp:39:27:39:30 | argv indirection | test.cpp:43:27:43:33 | access to array indirection |
 nodes
 | test.c:14:27:14:30 | argv indirection | semmle.label | argv indirection |
@@ -12,10 +14,15 @@ nodes
 | test.c:38:7:38:20 | globalUsername indirection | semmle.label | globalUsername indirection |
 | test.c:40:25:40:32 | username indirection | semmle.label | username indirection |
 | test.c:51:18:51:23 | query1 indirection | semmle.label | query1 indirection |
+| test.c:75:8:75:16 | gets output argument | semmle.label | gets output argument |
+| test.c:76:17:76:25 | userInput indirection | semmle.label | userInput indirection |
+| test.c:77:20:77:28 | userInput indirection | semmle.label | userInput indirection |
 | test.cpp:39:27:39:30 | argv indirection | semmle.label | argv indirection |
 | test.cpp:43:27:43:33 | access to array indirection | semmle.label | access to array indirection |
 subpaths
 #select
 | test.c:21:18:21:23 | query1 | test.c:14:27:14:30 | argv indirection | test.c:21:18:21:23 | query1 indirection | This argument to a SQL query function is derived from $@ and then passed to mysql_query(sqlArg). | test.c:14:27:14:30 | argv indirection | user input (a command-line argument) |
 | test.c:51:18:51:23 | query1 | test.c:14:27:14:30 | argv indirection | test.c:51:18:51:23 | query1 indirection | This argument to a SQL query function is derived from $@ and then passed to mysql_query(sqlArg). | test.c:14:27:14:30 | argv indirection | user input (a command-line argument) |
+| test.c:76:17:76:25 | userInput | test.c:75:8:75:16 | gets output argument | test.c:76:17:76:25 | userInput indirection | This argument to a SQL query function is derived from $@ and then passed to SQLPrepare(StatementText). | test.c:75:8:75:16 | gets output argument | user input (string read by gets) |
+| test.c:77:20:77:28 | userInput | test.c:75:8:75:16 | gets output argument | test.c:77:20:77:28 | userInput indirection | This argument to a SQL query function is derived from $@ and then passed to SQLExecDirect(StatementText). | test.c:75:8:75:16 | gets output argument | user input (string read by gets) |
 | test.cpp:43:27:43:33 | access to array | test.cpp:39:27:39:30 | argv indirection | test.cpp:43:27:43:33 | access to array indirection | This argument to a SQL query function is derived from $@ and then passed to pqxx::work::exec1((unnamed parameter 0)). | test.cpp:39:27:39:30 | argv indirection | user input (a command-line argument) |

cpp/ql/test/query-tests/Security/CWE/CWE-089/SqlTainted/test.c

@@ -50,3 +50,29 @@ void badFunc() {
   snprintf(query1, 1000, "SELECT UID FROM USERS where name = \"%s\"", userName);
   mysql_query(0, query1); // BAD
 }
+
+//ODBC Library Rountines
+typedef unsigned char SQLCHAR;
+typedef long int SQLINTEGER;
+typedef int SQLRETURN;
+typedef void* SQLHSTMT;
+
+char* gets(char *str);
+
+
+SQLRETURN SQLPrepare(  
+     SQLHSTMT      StatementHandle,  
+     SQLCHAR *     StatementText,  
+     SQLINTEGER    TextLength);  
+
+     SQLRETURN SQLExecDirect(  
+     SQLHSTMT     StatementHandle,  
+     SQLCHAR *    StatementText,  
+     SQLINTEGER   TextLength);  
+
+void ODBCTests(){
+  char userInput[100];
+  gets(userInput);
+  SQLPrepare(0, userInput, 100); // BAD
+  SQLExecDirect(0, userInput, 100); // BAD
+}