From ef070e191cb1569e0e217c489727ee93bff6cfc4 Mon Sep 17 00:00:00 2001 From: Michael Hohn Date: Thu, 5 Jun 2025 14:08:01 -0700 Subject: [PATCH] workshop discussion source --- SqlInjection.ql | 4 +- codeql-c-sqli.code-workspace | 3 +- models.ql | 43 +++++++++++++++++ models1.ql | 29 ++++++++++++ models2.ql | 68 +++++++++++++++++++++++++++ models3.ql | 74 ++++++++++++++++++++++++++++++ models4.ql | 84 ++++++++++++++++++++++++++++++++++ mycustom.qll | 17 +++++++ session.ql | 89 ++++++++++++++++++++++++++---------- 9 files changed, 385 insertions(+), 26 deletions(-) create mode 100644 models.ql create mode 100644 models1.ql create mode 100644 models2.ql create mode 100644 models3.ql create mode 100644 models4.ql create mode 100644 mycustom.qll diff --git a/SqlInjection.ql b/SqlInjection.ql index 47e8794..a19535f 100644 --- a/SqlInjection.ql +++ b/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() } diff --git a/codeql-c-sqli.code-workspace b/codeql-c-sqli.code-workspace index aa98b17..1645c61 100644 --- a/codeql-c-sqli.code-workspace +++ b/codeql-c-sqli.code-workspace @@ -6,6 +6,7 @@ ], "settings": { "codeQL.runningQueries.autoSave": true, - "makefile.configureOnOpen": false + "makefile.configureOnOpen": false, + "sarif-viewer.connectToGithubCodeScanning": "off" } } diff --git a/models.ql b/models.ql new file mode 100644 index 0000000..03b528e --- /dev/null +++ b/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 + diff --git a/models1.ql b/models1.ql new file mode 100644 index 0000000..3698d60 --- /dev/null +++ b/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() \ No newline at end of file diff --git a/models2.ql b/models2.ql new file mode 100644 index 0000000..2ea1cb0 --- /dev/null +++ b/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; + +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() + ")" diff --git a/models3.ql b/models3.ql new file mode 100644 index 0000000..6905dd3 --- /dev/null +++ b/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; + +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 + diff --git a/models4.ql b/models4.ql new file mode 100644 index 0000000..548c55f --- /dev/null +++ b/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; + +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" + } + } + diff --git a/mycustom.qll b/mycustom.qll new file mode 100644 index 0000000..5b1e89c --- /dev/null +++ b/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" + } + } \ No newline at end of file diff --git a/session.ql b/session.ql index f57f6da..de41b9d 100644 --- a/session.ql +++ b/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 \ No newline at end of file + 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; + +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*() \ No newline at end of file