Skip to content

Commit 84c9846

Browse files
committed
Catch security vulnerability thanks to custom CodeQL rule
1 parent e39d135 commit 84c9846

File tree

8 files changed

+97
-3
lines changed

8 files changed

+97
-3
lines changed

.github/codeql/codeql-config.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
queries:
2-
# - uses: ./codeql-queries/Sarge.qls
2+
- uses: ./codeql-queries/Joor.qls
33
- uses: java-security-and-quality

.github/codeql/golden.csv

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
Tool,Severity,Code,Description,Location,Line
1+
Tool,Code,Description,Location,Line
2+
CodeQL,java/code-injection,Tainted data passed to joor,main/java/com/moduscreate/java/security/codeql/App.java,18.0

README.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# java-security-codeql
22

3+
This repository is part of Tweag's experience sharing of using GitHub's
4+
[CodeQL](https://codeql.github.com/) static analysis tool. Other resources of ours include:
5+
6+
* A variant of this repository using Python as the programming language of choice: https://github.com/smelc/sarge-security-codeql
7+
* A technical blog post: [Getting started with CodeQL, GitHub's declarative static analyzer for security](LINK TBD)
8+
39
## Install the development environment
410

511
We provide a [.envrc](./envrc) file to enter the development shell automatically.
@@ -24,3 +30,11 @@ Trigger vulnerabilities in another terminal:
2430
```shell
2531
curl -X POST http://localhost:8080/api/invoke -d "method=sayHello"
2632
```
33+
34+
## Catching the vulnerability with CodeQL
35+
36+
The query to catch the example vulnerability is in [queries/JoorMain.ql](./codeql-queries/JoorMain.ql). This query
37+
relies on library code located in [queries/JoorLib.qll](./codeql-queries/JoorLib.qll). Debugging code, used
38+
to find out how to write the main query is in [queries/Scratch.ql](./codeql-queries/Scratch.ql): this file
39+
gives insight how one discovers what to write in a CodeQL query (the blog post linked above
40+
explains this process in details).

codeql-queries/GetRemoteFlowSource.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,5 @@
1212
import java
1313
import semmle.code.java.dataflow.FlowSources
1414

15-
from RemoteFlowSource src
15+
from ActiveThreatModelSource src
1616
select src, "Potential flow source"

codeql-queries/Joor.qls

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- query: ./JoorMain.ql

codeql-queries/JoorLib.qll

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* This file has the qll extension because it is meant to be imported.
3+
* ql files on the other hand, are meant to contain a query and so
4+
* cannot be imported.
5+
*
6+
* See https://codeql.github.com/docs/ql-language-reference/modules/#kinds-of-modules
7+
*/
8+
9+
import semmle.code.java.dataflow.FlowSources
10+
private import semmle.code.java.dataflow.internal.DataFlowPrivate
11+
12+
class JoorSink extends ArgumentNode {
13+
JoorSink() {
14+
exists(MethodCall mc |
15+
mc.getAnArgument() = this.asExpr() and
16+
mc.getMethod().getQualifiedName() = "org.joor.Reflect.call"
17+
)
18+
}
19+
}

codeql-queries/JoorMain.ql

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/**
2+
* @name Code injection
3+
* @description Interpreting unsanitized user input as code allows a malicious user to perform arbitrary
4+
* code execution.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 9.3
8+
* @sub-severity high
9+
* @precision high
10+
* @id java/code-injection
11+
* @tags security
12+
* external/cwe/cwe-094
13+
* external/cwe/cwe-095
14+
* external/cwe/cwe-116
15+
*/
16+
17+
import java
18+
import semmle.code.java.dataflow.FlowSources
19+
import JoorLib
20+
import JoorFlow::PathGraph // So that tainted paths render nicely in the CodeQL Query Results view
21+
22+
private module JoorConfig implements DataFlow::ConfigSig {
23+
predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource }
24+
25+
predicate isSink(DataFlow::Node sink) { sink instanceof JoorSink }
26+
}
27+
28+
module JoorFlow = TaintTracking::Global<JoorConfig>;
29+
30+
from JoorFlow::PathNode source, JoorFlow::PathNode sink
31+
where JoorFlow::flowPath(source, sink)
32+
select sink.getNode(), source, sink, "Tainted data passed to joor"

codeql-queries/Scratch.ql

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/**
2+
* @name GetRemoteFlowSource
3+
* @description Find all flow sources in the project
4+
* @kind problem
5+
* @tags correctness
6+
* @problem.severity recommendation
7+
* @sub-severity low
8+
* @precision very-high
9+
* @id py/get-remote-flow-source
10+
*/
11+
12+
import java
13+
import semmle.code.java.dataflow.FlowSources
14+
import semmle.code.java.dataflow.internal.DataFlowNodes
15+
private import semmle.code.java.dataflow.internal.DataFlowPrivate
16+
17+
// from DataFlow::Node node
18+
// where node.getLocation().getFile().toString() = "App"
19+
// and node.getLocation().getStartLine() = 18
20+
// // and node.asExpr().(MethodCall).getMethod().getQualifiedName() = "org.joor.Reflect.call"
21+
// select node, node.getAQlClass(), "node" //, node.asExpr().(MethodCall), node.asExpr().(MethodCall).getMethod().getQualifiedName(), "node"
22+
from DataFlow::Node node
23+
where
24+
node.getLocation().getFile().toString() = "App" and
25+
node.getLocation().getStartLine() = 18
26+
// select node, node.(ArgumentNode).getCall().asCall().getQualifier()., "node"
27+
select node, "node", node.(ArgumentNode).asExpr(), "(ArgumentNode).asExpr()"

0 commit comments

Comments
 (0)