diff --git a/.gitignore b/.gitignore index 4c3f7f3..aab9f69 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,5 @@ *~ *.html +*/codeql-pack.lock.yml +*.class +src/java-sqli-* diff --git a/README.org b/README.org index d5585a1..2d0a1eb 100644 --- a/README.org +++ b/README.org @@ -2,6 +2,18 @@ This is an introductory codeql workshop for Java. 1. The problem and its source code are in [[./src]] along with a full description in [[./src/README.org]]. - 2. The developed queries are in [[./solutions]] and their tests in [[./tests]]. The - derivation of those queries is described in [[./solutions/README.org]] + 2. The query is developed in [[./session/README.org]] +* Setup + #+BEGIN_SRC sh :session shared :results output + cd ~/local + git clone + + # CLI + cd ~/local/codeql-workshop-sql-injection-java + codeql pack install solutions + codeql pack install session + #+END_SRC + + For VS Code, use + : command palette > install pack dependencies diff --git a/codeql-workspace.yml b/codeql-workspace.yml new file mode 100644 index 0000000..d3c7345 --- /dev/null +++ b/codeql-workspace.yml @@ -0,0 +1,2 @@ +provides: + - "*/*.qlpack" diff --git a/java-demo.code-workspace b/java-demo.code-workspace index fd3b45f..7907a16 100644 --- a/java-demo.code-workspace +++ b/java-demo.code-workspace @@ -2,6 +2,10 @@ "folders": [ { "path": "." + }, + { + "name": "[java-sqli-ddb5e54 source archive]", + "uri": "codeql-zip-archive://0-82/Users/hohn/local/codeql-workshop-sql-injection-java/src/java-sqli-ddb5e54/src.zip" } ], "settings": { diff --git a/session/README.org b/session/README.org new file mode 100644 index 0000000..68b2397 --- /dev/null +++ b/session/README.org @@ -0,0 +1,118 @@ +* SQL injection example + This directory contains the codeql session snapshots as well as the full query + [[./full-query-old-style.ql]] + + The rest of this README contains a description of the query's development. + +** Develop the query bottom-up + 1. Identify the /source/ part of the + : System.console().readLine(); + expression, the =buf= argument. + Start from a =from..where..select=, then convert to a predicate. + + 2. Identify the /sink/ part of the + : conn.createStatement().executeUpdate(query); + expression, the =query= argument. Again start from =from..where..select=, + then convert to a predicate. + + 3. Fill in the /taintflow configuration/ boilerplate + #+BEGIN_SRC java + class SqliFlowConfig extends TaintTracking::Configuration { + SqliFlowConfig() { this = "SqliFlow" } + + override predicate isSource(DataFlow::Node node) { + none() + } + + override predicate isSink(DataFlow::Node node) { + none() + } + } + #+END_SRC + + The final query (without =isAdditionalTaintStep=) is + #+BEGIN_SRC java + /** + ,* @name SQLI Vulnerability + ,* @description Using untrusted strings in a sql query allows sql injection attacks. + ,* @kind path-problem + ,* @id java/SQLIVulnerable + ,* @problem.severity warning + ,*/ + + import java + import semmle.code.java.dataflow.TaintTracking + import DataFlow::PathGraph + + class SqliFlowConfig extends TaintTracking::Configuration { + SqliFlowConfig() { this = "SqliFlow" } + + override predicate isSource(DataFlow::Node source) { + // System.console().readLine(); + exists(Call read | + read.getCallee().getName() = "readLine" and + read = source.asExpr() + ) + } + + override predicate isSink(DataFlow::Node sink) { + // conn.createStatement().executeUpdate(query); + exists(Call exec | + exec.getCallee().getName() = "executeUpdate" and + exec.getArgument(0) = sink.asExpr() + ) + } + } + + from SqliFlowConfig conf, DataFlow::PathNode source, DataFlow::PathNode sink + where conf.hasFlowPath(source, sink) + select sink, source, sink, "Possible SQL injection" + #+END_SRC + +** (optional) Review of the results via SARIF file + Query results are available in several output formats using the cli. The + following produces the sarif format, a json-based result description. + + #+BEGIN_SRC sh + # The setup information from before + SRCDIR=$HOME/local/codeql-training-material.java-sqli/java/codeql-dataflow-sql-injection + DB=$SRCDIR/java-sqli-$(cd $SRCDIR && git rev-parse --short HEAD) + + # Check paths + echo $DB + echo $SRCDIR + + # To see the help + codeql database analyze -h + + # Run a query + codeql database analyze \ + -v \ + --ram=14000 \ + -j12 \ + --rerun \ + --search-path ~/local/vmsync/ql \ + --format=sarif-latest \ + --output java-sqli.sarif \ + -- \ + $DB \ + $SRCDIR/SqlInjection.ql + + # Examine the file in an editor + edit java-sqli.sarif + #+END_SRC + + An example of using the sarif data is in the the jq script [[./sarif-summary.jq]]. + When run against the sarif input via + #+BEGIN_SRC sh + jq --raw-output --join-output -f sarif-summary.jq < java-sqli.sarif > java-sqli.txt + #+END_SRC + it produces output in a form close to that of compiler error messages: + #+BEGIN_SRC text + query-id: message line + Path + ... + Path + ... + #+END_SRC + diff --git a/session/full-query-old-style.ql b/session/full-query-old-style.ql new file mode 100644 index 0000000..15638c4 --- /dev/null +++ b/session/full-query-old-style.ql @@ -0,0 +1,44 @@ +/** + * @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 java +import semmle.code.java.dataflow.TaintTracking +import DataFlow::PathGraph + +class SqliFlowConfig extends TaintTracking::Configuration { + SqliFlowConfig() { this = "SqliFlow" } + + override predicate isSource(DataFlow::Node source) { + // System.console().readLine(); + exists(Call read | + read.getCallee().getName() = "readLine" and + read = source.asExpr() + ) + } + + override predicate isSanitizer(DataFlow::Node sanitizer) { none() } + + override predicate isAdditionalTaintStep(DataFlow::Node into, DataFlow::Node out) { + // Extra taint step + // String.format("INSERT INTO users VALUES (%d, '%s')", id, info); + // Not needed here, but may be needed for larger libraries. + none() + } + + override predicate isSink(DataFlow::Node sink) { + // conn.createStatement().executeUpdate(query); + exists(Call exec | + exec.getCallee().getName() = "executeUpdate" and + exec.getArgument(0) = sink.asExpr() + ) + } +} + +from SqliFlowConfig conf, DataFlow::PathNode source, DataFlow::PathNode sink +where conf.hasFlowPath(source, sink) +select sink, source, sink, "Possible SQL injection" diff --git a/session/qlpack.yml b/session/qlpack.yml new file mode 100644 index 0000000..7820d79 --- /dev/null +++ b/session/qlpack.yml @@ -0,0 +1,6 @@ +--- +library: false +name: codeql-workshop/java-sql-injection +version: 0.0.1 +dependencies: + codeql/java-all: "*" diff --git a/solutions/qlpack.yml b/solutions/qlpack.yml index 7820d79..bdaef0c 100644 --- a/solutions/qlpack.yml +++ b/solutions/qlpack.yml @@ -1,6 +1,6 @@ --- library: false -name: codeql-workshop/java-sql-injection +name: codeql-workshop/java-sql-injection-solutions version: 0.0.1 dependencies: codeql/java-all: "*" diff --git a/src/README.org b/src/README.org index f0ae073..fa99fc0 100644 --- a/src/README.org +++ b/src/README.org @@ -1,4 +1,12 @@ * SQL injection example + This directory contains the problematic Java source code. The rest of this + README describes + - the [[*Setup and sample run][Setup and sample run]] for the problem, + - briefly describes how to [[*Identify the problem][Identify the problem]] and + - instructions to [[*Build the codeql database][Build the codeql database]] + + The codeql query is developed in [[../session/README.org]]. + ** Setup and sample run The jdbc connector at https://github.com/xerial/sqlite-jdbc, from [[https://github.com/xerial/sqlite-jdbc/releases/download/3.36.0.1/sqlite-jdbc-3.36.0.1.jar][here]] is included in the git repository. @@ -51,12 +59,11 @@ We write codeql to identify these two, and then connect them via - a /dataflow configuration/ -- for this problem, the more general /taintflow configuration/. - -** Build codeql database + +** Build the codeql database To get started, build the codeql database (adjust paths to your setup): #+BEGIN_SRC sh # Build the db with source commit id. - export PATH=$HOME/local/vmsync/codeql250:"$PATH" SRCDIR=$(pwd) DB=$SRCDIR/java-sqli-$(cd $SRCDIR && git rev-parse --short HEAD) @@ -72,8 +79,7 @@ Then add this database directory to your VS Code =DATABASES= tab. - -** Build codeql database in steps +** (old / optional) Build the codeql database in steps For larger projects, using a single command to build everything is costly when any part of the build fails. @@ -103,116 +109,4 @@ Then add this database directory to your VS Code =DATABASES= tab. -** Develop the query bottom-up - 1. Identify the /source/ part of the - : System.console().readLine(); - expression, the =buf= argument. - Start from a =from..where..select=, then convert to a predicate. - - 2. Identify the /sink/ part of the - : conn.createStatement().executeUpdate(query); - expression, the =query= argument. Again start from =from..where..select=, - then convert to a predicate. - - 3. Fill in the /taintflow configuration/ boilerplate - #+BEGIN_SRC java - class SqliFlowConfig extends TaintTracking::Configuration { - SqliFlowConfig() { this = "SqliFlow" } - - override predicate isSource(DataFlow::Node node) { - none() - } - - override predicate isSink(DataFlow::Node node) { - none() - } - } - #+END_SRC - - The final query (without =isAdditionalTaintStep=) is - #+BEGIN_SRC java - /** - ,* @name SQLI Vulnerability - ,* @description Using untrusted strings in a sql query allows sql injection attacks. - ,* @kind path-problem - ,* @id java/SQLIVulnerable - ,* @problem.severity warning - ,*/ - - import java - import semmle.code.java.dataflow.TaintTracking - import DataFlow::PathGraph - - class SqliFlowConfig extends TaintTracking::Configuration { - SqliFlowConfig() { this = "SqliFlow" } - - override predicate isSource(DataFlow::Node source) { - // System.console().readLine(); - exists(Call read | - read.getCallee().getName() = "readLine" and - read = source.asExpr() - ) - } - - override predicate isSink(DataFlow::Node sink) { - // conn.createStatement().executeUpdate(query); - exists(Call exec | - exec.getCallee().getName() = "executeUpdate" and - exec.getArgument(0) = sink.asExpr() - ) - } - } - - from SqliFlowConfig conf, DataFlow::PathNode source, DataFlow::PathNode sink - where conf.hasFlowPath(source, sink) - select sink, source, sink, "Possible SQL injection" - #+END_SRC - -** Optional: sarif file review of the results - Query results are available in several output formats using the cli. The - following produces the sarif format, a json-based result description. - - #+BEGIN_SRC sh - # The setup information from before - export PATH=$HOME/local/vmsync/codeql250:"$PATH" - SRCDIR=$HOME/local/codeql-training-material.java-sqli/java/codeql-dataflow-sql-injection - DB=$SRCDIR/java-sqli-$(cd $SRCDIR && git rev-parse --short HEAD) - - # Check paths - echo $DB - echo $SRCDIR - - # To see the help - codeql database analyze -h - - # Run a query - codeql database analyze \ - -v \ - --ram=14000 \ - -j12 \ - --rerun \ - --search-path ~/local/vmsync/ql \ - --format=sarif-latest \ - --output java-sqli.sarif \ - -- \ - $DB \ - $SRCDIR/SqlInjection.ql - - # Examine the file in an editor - edit java-sqli.sarif - #+END_SRC - - An example of using the sarif data is in the the jq script [[./sarif-summary.jq]]. - When run against the sarif input via - #+BEGIN_SRC sh - jq --raw-output --join-output -f sarif-summary.jq < java-sqli.sarif > java-sqli.txt - #+END_SRC - it produces output in a form close to that of compiler error messages: - #+BEGIN_SRC text - query-id: message line - Path - ... - Path - ... - #+END_SRC diff --git a/src/SqlInjection.ql b/src/SqlInjection.ql deleted file mode 100644 index 7d5fac6..0000000 --- a/src/SqlInjection.ql +++ /dev/null @@ -1,44 +0,0 @@ -/** - * @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 java -import semmle.code.java.dataflow.TaintTracking -import DataFlow::PathGraph - -class SqliFlowConfig extends TaintTracking::Configuration { - SqliFlowConfig() { this = "SqliFlow" } - - override predicate isSource(DataFlow::Node source) { - // System.console().readLine(); - exists(Call read | - read.getCallee().getName() = "readLine" and - read = source.asExpr() - ) - } - - override predicate isSanitizer(DataFlow::Node sanitizer) { none() } - - override predicate isAdditionalTaintStep(DataFlow::Node into, DataFlow::Node out) { - // Extra taint step - // String.format("INSERT INTO users VALUES (%d, '%s')", id, info); - // Not needed here, but may be needed for larger libraries. - none() - } - - override predicate isSink(DataFlow::Node sink) { - // conn.createStatement().executeUpdate(query); - exists(Call exec | - exec.getCallee().getName() = "executeUpdate" and - exec.getArgument(0) = sink.asExpr() - ) - } -} - -from SqliFlowConfig conf, DataFlow::PathNode source, DataFlow::PathNode sink -where conf.hasFlowPath(source, sink) -select sink, source, sink, "Possible SQL injection"