workshop discussion source

SqlInjection.ql

@@ -15,8 +15,8 @@ module SqliFlowConfig implements DataFlow::ConfigSig {
         // count = read(STDIN_FILENO, buf, BUFSIZE);
         exists(FunctionCall read |
             read.getTarget().getName() = "read" and
-            read.getArgument(1) = source.(DataFlow::PostUpdateNode).getPreUpdateNode().asIndirectArgument()
-        )
+            read.getArgument(1) = source.asDefiningArgument()
+                    )
     }
 
     predicate isBarrier(DataFlow::Node sanitizer) { none() }

codeql-c-sqli.code-workspace

@@ -6,6 +6,7 @@
 	],
 	"settings": {
         "codeQL.runningQueries.autoSave": true,
-        "makefile.configureOnOpen": false
+        "makefile.configureOnOpen": false,
+        "sarif-viewer.connectToGithubCodeScanning": "off"
 	}
 }

models.ql

@@ -0,0 +1,43 @@
+import cpp
+
+import semmle.code.cpp.models.Models
+import semmle.code.cpp.commons.Scanf
+
+import semmle.code.cpp.models.implementations.Strcpy
+
+
+/*
+ * sources. To use this QL library, create a QL class extending `DataFlowFunction` with a
+ * characteristic predicate that selects the function or set of functions you
+ * are modeling. Within that class, override the predicates provided by
+ * `RemoteFlowSourceFunction` or `RemoteFlowSinkFunction` to match the flow within that 
+ */
+class SDF extends DataFlowFunction {
+    // see import semmle.code.cpp.models.implementations.Strcpy
+    SDF () { any() }
+  
+    override predicate hasDataFlow(FunctionInput input, FunctionOutput output) { any()}
+
+}
+ 
+
+import semmle.code.cpp.models.implementations.Fread
+// Fread extends RemoteFlowSourceFunction 
+
+import semmle.code.cpp.models.interfaces.DataFlow
+import semmle.code.cpp.models.interfaces.FlowSource
+import semmle.code.cpp.models.implementations.Recv
+// See
+// private class Recv extends AliasFunction, ArrayFunction, SideEffectFunction,
+//   RemoteFlowSourceFunction
+// for "read"
+
+// // Find all *definitions*
+// from DataFlowFunction dff 
+// select dff
+
+// Find *uses* (via Call)
+from DataFlowFunction dff, Call cl
+where cl.getTarget() = dff
+select cl.getLocation().getFile(), cl, dff
+

models1.ql

@@ -0,0 +1,29 @@
+import cpp
+import semmle.code.cpp.models.Models
+import semmle.code.cpp.models.interfaces.FlowSource
+
+// get sources / sinks from stdlib, use in our flow
+
+// from RemoteFlowSourceFunction rfs,   FunctionOutput output, string description
+// where rfs.hasRemoteFlowSource(output, description)
+// select rfs, rfs.getACallToThisFunction(), output, description
+
+import semmle.code.cpp.models.interfaces.Sql
+
+from SqlExecutionFunction sef
+select sef, sef.getACallToThisFunction()
+
+
+class Foo extends Expr {
+    Foo () { this.getNumChild() = 1 }
+    Expr getTheChild() {
+        result = this.getChild(0)
+        // given set f, in python:  {element.getChild(0) for element in f}
+    }
+}
+
+// from Foo f 
+// select f, f.getTheChild()
+
+// from BinaryOperation bin
+// select bin, bin.getAChild()

models2.ql

@@ -0,0 +1,68 @@
+/**
+ * @name Uncontrolled data in SQL query
+ * @description Including user-supplied data in a SQL query without
+ *              neutralizing special elements can make code vulnerable
+ *              to SQL Injection.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 8.8
+ * @precision high
+ * @id cpp/sql-injection
+ * @tags security
+ *       external/cwe/cwe-089
+ */
+
+import cpp
+import semmle.code.cpp.security.Security
+import semmle.code.cpp.security.FlowSources
+import semmle.code.cpp.security.FunctionWithWrappers
+import semmle.code.cpp.ir.IR
+import semmle.code.cpp.ir.dataflow.TaintTracking
+import SqlTainted::PathGraph
+
+class SqlLikeFunction extends FunctionWithWrappers {
+  SqlLikeFunction() { sqlArgument(this.getName(), _) }
+
+  override predicate interestingArg(int arg) { sqlArgument(this.getName(), arg) }
+}
+
+Expr asSinkExpr(DataFlow::Node node) {
+  result = node.asIndirectArgument()
+  or
+  // We want the conversion so we only get one node for the expression
+  result = node.asExpr()
+}
+
+module SqlTaintedConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof FlowSource }
+
+  predicate isSink(DataFlow::Node node) {
+    exists(SqlLikeFunction runSql | runSql.outermostWrapperFunctionCall(asSinkExpr(node), _))
+  }
+
+  predicate isBarrier(DataFlow::Node node) {
+    node.asExpr().getUnspecifiedType() instanceof IntegralType
+  }
+
+  predicate isBarrierIn(DataFlow::Node node) {
+    exists(SqlBarrierFunction sql, int arg, FunctionInput input |
+      node.asIndirectArgument() = sql.getACallToThisFunction().getArgument(arg) and
+      input.isParameterDeref(arg) and
+      sql.barrierSqlArgument(input, _)
+    )
+  }
+}
+
+module SqlTainted = TaintTracking::Global<SqlTaintedConfig>;
+
+from
+  SqlLikeFunction runSql, Expr taintedArg, FlowSource taintSource, SqlTainted::PathNode sourceNode,
+  SqlTainted::PathNode sinkNode, string callChain
+where
+  runSql.outermostWrapperFunctionCall(taintedArg, callChain) and
+  SqlTainted::flowPath(sourceNode, sinkNode) and
+  taintedArg = asSinkExpr(sinkNode.getNode()) and
+  taintSource = sourceNode.getNode()
+select taintedArg, sourceNode, sinkNode,
+  "This argument to a SQL query function is derived from $@ and then passed to " + callChain + ".",
+  taintSource, "user input (" + taintSource.getSourceType() + ")"

models3.ql

@@ -0,0 +1,74 @@
+/**
+ * @name Uncontrolled data in SQL query
+ * @description Including user-supplied data in a SQL query without
+ *              neutralizing special elements can make code vulnerable
+ *              to SQL Injection.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 8.8
+ * @precision high
+ * @id cpp/sql-injection
+ * @tags security
+ *       external/cwe/cwe-089
+ */
+
+import cpp
+import semmle.code.cpp.security.Security
+import semmle.code.cpp.security.FlowSources
+import semmle.code.cpp.security.FunctionWithWrappers
+import semmle.code.cpp.ir.IR
+import semmle.code.cpp.ir.dataflow.TaintTracking
+import SqlTainted::PathGraph
+
+class SqlLikeFunction extends FunctionWithWrappers {
+  SqlLikeFunction() { sqlArgument(this.getName(), _) }
+
+  override predicate interestingArg(int arg) { sqlArgument(this.getName(), arg) }
+}
+
+Expr asSinkExpr(DataFlow::Node node) {
+  result = node.asIndirectArgument()
+  or
+  // We want the conversion so we only get one node for the expression
+  result = node.asExpr()
+}
+
+module SqlTaintedConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof FlowSource }
+
+  predicate isSink(DataFlow::Node node) {
+    exists(SqlLikeFunction runSql | runSql.outermostWrapperFunctionCall(asSinkExpr(node), _))
+  }
+
+  predicate isBarrier(DataFlow::Node node) {
+    node.asExpr().getUnspecifiedType() instanceof IntegralType
+  }
+
+  predicate isBarrierIn(DataFlow::Node node) {
+    exists(SqlBarrierFunction sql, int arg, FunctionInput input |
+      node.asIndirectArgument() = sql.getACallToThisFunction().getArgument(arg) and
+      input.isParameterDeref(arg) and
+      sql.barrierSqlArgument(input, _)
+    )
+  }
+}
+
+module SqlTainted = TaintTracking::Global<SqlTaintedConfig>;
+
+from
+  SqlLikeFunction runSql, Expr taintedArg, FlowSource taintSource, SqlTainted::PathNode sourceNode,
+  SqlTainted::PathNode sinkNode, string callChain
+where
+  runSql.outermostWrapperFunctionCall(taintedArg, callChain) and
+  SqlTainted::flowPath(sourceNode, sinkNode) and
+  taintedArg = asSinkExpr(sinkNode.getNode()) and
+  taintSource = sourceNode.getNode()
+select taintedArg, sourceNode, sinkNode,
+  "This argument to a SQL query function is derived from $@ and then passed to " + callChain + ".",
+  taintSource, "user input (" + taintSource.getSourceType() + ")"
+
+// add 
+  // char* get_user_info() 
+
+  import mycustom
+

models4.ql

@@ -0,0 +1,84 @@
+/**
+ * @name Uncontrolled data in SQL query
+ * @description Including user-supplied data in a SQL query without
+ *              neutralizing special elements can make code vulnerable
+ *              to SQL Injection.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 8.8
+ * @precision high
+ * @id cpp/sql-injection
+ * @tags security
+ *       external/cwe/cwe-089
+ */
+
+import cpp
+import semmle.code.cpp.security.Security
+import semmle.code.cpp.security.FlowSources
+import semmle.code.cpp.security.FunctionWithWrappers
+import semmle.code.cpp.ir.IR
+import semmle.code.cpp.ir.dataflow.TaintTracking
+import SqlTainted::PathGraph
+
+class SqlLikeFunction extends FunctionWithWrappers {
+  SqlLikeFunction() { sqlArgument(this.getName(), _) }
+
+  override predicate interestingArg(int arg) { sqlArgument(this.getName(), arg) }
+}
+
+Expr asSinkExpr(DataFlow::Node node) {
+  result = node.asIndirectArgument()
+  or
+  // We want the conversion so we only get one node for the expression
+  result = node.asExpr()
+}
+
+module SqlTaintedConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof FlowSource }
+
+  predicate isSink(DataFlow::Node node) {
+    exists(SqlLikeFunction runSql | runSql.outermostWrapperFunctionCall(asSinkExpr(node), _))
+  }
+
+  predicate isBarrier(DataFlow::Node node) {
+    node.asExpr().getUnspecifiedType() instanceof IntegralType
+  }
+
+  predicate isBarrierIn(DataFlow::Node node) {
+    exists(SqlBarrierFunction sql, int arg, FunctionInput input |
+      node.asIndirectArgument() = sql.getACallToThisFunction().getArgument(arg) and
+      input.isParameterDeref(arg) and
+      sql.barrierSqlArgument(input, _)
+    )
+  }
+}
+
+module SqlTainted = TaintTracking::Global<SqlTaintedConfig>;
+
+from
+  SqlLikeFunction runSql, Expr taintedArg, FlowSource taintSource, SqlTainted::PathNode sourceNode,
+  SqlTainted::PathNode sinkNode, string callChain
+where
+  runSql.outermostWrapperFunctionCall(taintedArg, callChain) and
+  SqlTainted::flowPath(sourceNode, sinkNode) and
+  taintedArg = asSinkExpr(sinkNode.getNode()) and
+  taintSource = sourceNode.getNode()
+select taintedArg, sourceNode, sinkNode,
+  "This argument to a SQL query function is derived from $@ and then passed to " + callChain + ".",
+  taintSource, "user input (" + taintSource.getSourceType() + ")"
+
+// add 
+  // char* get_user_info() 
+
+  class MyFS extends LocalFlowSource { 
+    MyFS () {
+        exists(ReturnStmt rs |
+            rs.getEnclosingFunction().getName() = "get_user_info"
+            and this.asExpr() = rs.getExpr()
+        )
+    }
+    override string getSourceType() {
+        result = "foo"
+    }
+  }
+

mycustom.qll

@@ -0,0 +1,17 @@
+
+
+import cpp
+import semmle.code.cpp.security.Security
+import semmle.code.cpp.security.FlowSources
+
+  class MyFS extends LocalFlowSource { 
+    MyFS () {
+        exists(ReturnStmt rs |
+            rs.getEnclosingFunction().getName() = "get_user_info"
+            and this.asExpr() = rs.getExpr()
+        )
+    }
+    override string getSourceType() {
+        result = "foo"
+    }
+  }

session.ql

@@ -1,19 +1,23 @@
+/**
+ * @name SQLI Vulnerability
+ * @description Using untrusted strings in a sql query allows sql injection attacks.
+ * @kind path-problem
+ * @id cpp/sqlivulnerable
+ * @problem.severity warning
+ */
+
 import cpp
 
-// from Call cl 
+// from Call cl
 // select cl
-
-
 /*
-
-int get_new_id() {
-    int id = getpid();
-    return id;
-}
-*/
+ * int get_new_id() {
+ *    int id = getpid();
+ *    return id;
+ * }
+ */
 
 // Goal: Find connection
-
 // 1. reading user data -- source
 //    count = read(STDIN_FILENO, buf, BUFSIZE - 1);
 //                               ^^^
@@ -21,20 +25,27 @@ int get_new_id() {
 // where read.getTarget().getName() = "read"
 // and buf = read.getArgument(1)
 // select read, buf
-
 predicate isSource(Expr buf) {
-    exists(FunctionCall read |
-         read.getTarget().getName() = "read"
-         and buf = read.getArgument(1)
-    )
+  exists(FunctionCall read |
+    read.getTarget().getName() = "read" and
+    buf = read.getArgument(1)
+  )
 }
 
 // from Expr buf
 // where isSource(buf)
 // select buf
+class MySource extends Expr {
+  MySource() {
+    exists(FunctionCall read |
+      read.getTarget().getName() = "read" and
+      this = read.getArgument(1) 
+    )
+  }
+}
 
-
-
+// from MySource buf
+// select buf
 // 2. writing sql -- sink
 //     rc = sqlite3_exec(db, query, NULL, 0, &zErrMsg);
 //                           ^^^^^
@@ -43,14 +54,46 @@ predicate isSource(Expr buf) {
 // and query = exec.getArgument(1)
 // select exec, query
 predicate isSink(Expr query) {
-    exists(FunctionCall exec |
-        exec.getTarget().getName() = "sqlite3_exec"
-        and query = exec.getArgument(1)
-    )
+  exists(FunctionCall exec |
+    exec.getTarget().getName() = "sqlite3_exec" and
+    query = exec.getArgument(1)
+  )
 }
-// from Expr query 
+
+// from Expr query
 // where isSink(query)
 // select query
+class MySink extends Expr {
+  FunctionCall exec;
 
+  MySink() {
+    exec.getTarget().getName() = "sqlite3_exec" and
+    this = exec.getArgument(1)
+  }
 
-// 3. find call path between 1 and 2 them
+  FunctionCall getTheCall() { result = exec }
+}
+
+// from MySink query
+// select query, query.getTheCall()
+// 3. find call path between 1 and 2 them
+import semmle.code.cpp.dataflow.new.TaintTracking
+
+module SqliFlowConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    exists(MySource ms | ms = source.asDefiningArgument())
+  }
+
+  predicate isSink(DataFlow::Node sink) { exists(MySink ms | ms = sink.asIndirectArgument()) }
+}
+
+module MyFlow = TaintTracking::Global<SqliFlowConfig>;
+
+import MyFlow::PathGraph
+
+from MyFlow::PathNode source, MyFlow::PathNode sink
+where MyFlow::flowPath(source, sink)
+select sink, source, sink, "Possible SQL injection"
+
+// from MySource ms 
+// select ms.getASuccessor*()