Skip to content

Commit d9b0b67

Browse files
author
Alvaro Muñoz
authored
Merge pull request #19 from GitHubSecurityLab/new_java_queries
Add new Java queries and CVEs
2 parents 1fde8dd + 955dc49 commit d9b0b67

File tree

12 files changed

+472
-33
lines changed

12 files changed

+472
-33
lines changed

‎java/lib/github/.gitkeep

Whitespace-only changes.

‎java/lib/github/BeanManipulation.qll

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
private import semmle.code.java.dataflow.TaintTracking
2+
3+
class SetPropertyMethod extends Method {
4+
SetPropertyMethod() {
5+
this.getDeclaringType()
6+
.getASourceSupertype*()
7+
.hasQualifiedName("org.springframework.beans", "PropertyAccessor") and
8+
this.hasName(["setPropertyValue", "setPropertyValues"])
9+
or
10+
this.getDeclaringType()
11+
.getASourceSupertype*()
12+
.hasQualifiedName(["org.apache.commons.beanutils", "org.apache.commons.beanutils2"],
13+
["PropertyUtils", "PropertyUtilsBean"]) and
14+
this.hasName(["setProperty", "setNestedProperty", "setSimpleProperty"])
15+
or
16+
this.getDeclaringType()
17+
.getASourceSupertype*()
18+
.hasQualifiedName(["org.apache.commons.beanutils", "org.apache.commons.beanutils2"],
19+
["BeanUtils", "BeanUtilsBean"]) and
20+
this.hasName(["setProperty", "populate"])
21+
or
22+
this.getDeclaringType()
23+
.getASourceSupertype*()
24+
.hasQualifiedName("org.springframework.data.redis.hash", "BeanUtilsHashMapper") and
25+
this.hasName("fromHash")
26+
or
27+
this.getDeclaringType()
28+
.getASourceSupertype*()
29+
.hasQualifiedName("org.springframework.beans",
30+
"AbstractNestablePropertyAccessor$PropertyHandler") and
31+
this.hasName("setValue")
32+
or
33+
this.getDeclaringType()
34+
.getASourceSupertype*()
35+
.hasQualifiedName("org.springframework.beans",
36+
["AbstractNestablePropertyAccessor", "AbstractPropertyAccessor"]) and
37+
this.hasName(["setPropertyValue", "setPropertyValues"])
38+
}
39+
}
40+
41+
class BeanManipulationSink extends DataFlow::ExprNode {
42+
BeanManipulationSink() {
43+
exists(MethodAccess ma |
44+
ma.getMethod() instanceof SetPropertyMethod and
45+
this.getExpr() = ma.getAnArgument()
46+
)
47+
}
48+
}

‎java/src/CVEs/CVE-2021-44228.ql

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,12 @@
1111
import java
1212
import semmle.code.java.dataflow.TaintTracking
1313
import semmle.code.java.security.JndiInjection
14-
import DataFlow::PathGraph
14+
import Log4ShellFlow::PathGraph
1515

16-
class Log4ShellConfig extends TaintTracking::Configuration {
17-
Log4ShellConfig() { this = "Log4ShellConfig" }
16+
private module Log4ShellConfig implements DataFlow::ConfigSig {
17+
int fieldFlowBranchLimit() { result = 9 }
1818

19-
override int fieldFlowBranchLimit() { result = 9 }
20-
21-
override predicate isSource(DataFlow::Node source) {
19+
predicate isSource(DataFlow::Node source) {
2220
exists(Method m, Parameter p |
2321
m.getParameter(0) = p and
2422
p = source.asParameter() and
@@ -28,9 +26,9 @@ class Log4ShellConfig extends TaintTracking::Configuration {
2826
)
2927
}
3028

31-
override predicate isSink(DataFlow::Node sink) { sink instanceof JndiInjectionSink }
29+
predicate isSink(DataFlow::Node sink) { sink instanceof JndiInjectionSink }
3230

33-
override predicate isSanitizer(DataFlow::Node n) {
31+
predicate isBarrier(DataFlow::Node n) {
3432
exists(string s | n.getLocation().getFile().getBaseName() = s |
3533
s.matches("%Appender.java") and not s = "AbstractOutputStreamAppender.java"
3634
or
@@ -41,6 +39,8 @@ class Log4ShellConfig extends TaintTracking::Configuration {
4139
}
4240
}
4341

44-
from Log4ShellConfig conf, DataFlow::PathNode source, DataFlow::PathNode sink
45-
where conf.hasFlowPath(source, sink)
46-
select sink, source, sink, "log4shell"
42+
module Log4ShellFlow = TaintTracking::Global<Log4ShellConfig>;
43+
44+
from Log4ShellFlow::PathNode source, Log4ShellFlow::PathNode sink
45+
where Log4ShellFlow::flowPath(source, sink)
46+
select sink, source, sink, "Log4shell"

‎java/src/CVEs/CVE-2022-22965.ql

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/**
2+
* @name Spring4Shell
3+
* @description ClassLoader Manipulation in Spring Beans
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @precision high
7+
* @id githubsecuritylab/spring4shell
8+
* @tags security
9+
*/
10+
11+
import java
12+
import semmle.code.java.dataflow.FlowSources
13+
import Spring4ShellFlow::PathGraph
14+
import github.BeanManipulation
15+
16+
private module Spring4ShellConfig implements DataFlow::ConfigSig {
17+
predicate isSource(DataFlow::Node source) {
18+
source instanceof RemoteFlowSource and
19+
source.asExpr().getEnclosingCallable().getDeclaringType().hasName("WebUtils")
20+
}
21+
22+
predicate isSink(DataFlow::Node sink) { sink instanceof BeanManipulationSink }
23+
24+
predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
25+
exists(ConstructorCall ma |
26+
ma.getConstructor().getDeclaringType().getASourceSupertype*().hasName("MutablePropertyValues") and
27+
n1.asExpr() = ma.getAnArgument() and
28+
n2.(DataFlow::PostUpdateNode).getPreUpdateNode() = DataFlow::getInstanceArgument(ma)
29+
)
30+
}
31+
32+
predicate isBarrier(DataFlow::Node n) {
33+
n.getLocation().getFile().getRelativePath().matches(["%test%", "%mock%"])
34+
or
35+
exists(MethodAccess ma |
36+
ma.getMethod().hasName("toString") and DataFlow::getInstanceArgument(ma) = n
37+
)
38+
}
39+
}
40+
41+
module Spring4ShellFlow = TaintTracking::Global<Spring4ShellConfig>;
42+
43+
from Spring4ShellFlow::PathNode source, Spring4ShellFlow::PathNode sink
44+
where Spring4ShellFlow::flowPath(source, sink)
45+
select sink, source, sink, "Bean Manipulation at $@.", sink.getNode(), "user input"

‎java/src/CVEs/CVE-2022-33980.ql

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,12 @@
1010

1111
import java
1212
import semmle.code.java.dataflow.FlowSources
13-
import semmle.code.java.dataflow.TaintTracking
14-
import DataFlow
15-
import DataFlow::PathGraph
13+
import ACCInjectionFlow::PathGraph
1614

17-
class InterpolatorConfig extends TaintTracking::Configuration {
18-
InterpolatorConfig() { this = "InterpolatorConfig" }
15+
private module ACCInjectionConfig implements DataFlow::ConfigSig {
16+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
1917

20-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
21-
22-
override predicate isSink(DataFlow::Node sink) {
18+
predicate isSink(DataFlow::Node sink) {
2319
exists(MethodAccess ma, Method m | ma.getMethod() = m and ma.getAnArgument() = sink.asExpr() |
2420
m.getName() = ["addProperty", "setProperty"] and
2521
m.getDeclaringType()
@@ -38,6 +34,8 @@ class InterpolatorConfig extends TaintTracking::Configuration {
3834
}
3935
}
4036

41-
from DataFlow::PathNode source, DataFlow::PathNode sink, InterpolatorConfig conf
42-
where conf.hasFlowPath(source, sink)
43-
select sink.getNode(), source, sink, "injection from $@.", source.getNode(), "this user input"
37+
module ACCInjectionFlow = TaintTracking::Global<ACCInjectionConfig>;
38+
39+
from ACCInjectionFlow::PathNode source, ACCInjectionFlow::PathNode sink
40+
where ACCInjectionFlow::flowPath(source, sink)
41+
select sink.getNode(), source, sink, "Script injection"

‎java/src/CVEs/CVE-2022-42889.ql

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,12 @@
1010

1111
import java
1212
import semmle.code.java.dataflow.FlowSources
13-
import semmle.code.java.dataflow.TaintTracking
14-
import DataFlow
15-
import DataFlow::PathGraph
13+
import ACTInjectionFlow::PathGraph
1614

17-
class InterpolatorConfig extends TaintTracking::Configuration {
18-
InterpolatorConfig() { this = "InterpolatorConfig" }
15+
private module ACTInjectionConfig implements DataFlow::ConfigSig {
16+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
1917

20-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
21-
22-
override predicate isSink(DataFlow::Node sink) {
18+
predicate isSink(DataFlow::Node sink) {
2319
exists(MethodAccess ma, Method m | ma.getMethod() = m and ma.getAnArgument() = sink.asExpr() |
2420
m.getName() = "replace" and
2521
m.getDeclaringType()
@@ -34,6 +30,8 @@ class InterpolatorConfig extends TaintTracking::Configuration {
3430
}
3531
}
3632

37-
from DataFlow::PathNode source, DataFlow::PathNode sink, InterpolatorConfig conf
38-
where conf.hasFlowPath(source, sink)
39-
select sink.getNode(), source, sink, "injection from $@.", source.getNode(), "this user input"
33+
module ACTInjectionFlow = TaintTracking::Global<ACTInjectionConfig>;
34+
35+
from ACTInjectionFlow::PathNode source, ACTInjectionFlow::PathNode sink
36+
where ACTInjectionFlow::flowPath(source, sink)
37+
select sink.getNode(), source, sink, "Script injection"

‎java/src/security/CWE-022/UnsafeURICheck.ql

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/**
2+
* @name Unsafe URI Check
3+
* @description Checking a URL against an allow/block list in Java may be unsafe.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @precision high
7+
* @id githubsecuritylab/unsafe-uri-check
8+
* @tags security
9+
* external/cwe/cwe-22
10+
*/
11+
12+
import java
13+
import semmle.code.java.dataflow.FlowSources
14+
import UnsafeURICheckFlow::PathGraph
15+
16+
// Reference: https://mail-archives.apache.org/mod_mbox/ambari-user/202102.mbox/%3CCAEJYuxEQZ_aPwJdAaSxPu-Dva%3Dhc7zZUx3-pzBORbd23g%2BGH1A%40mail.gmail.com%3E
17+
class ServletFilterInterface extends Interface {
18+
ServletFilterInterface() { this.hasQualifiedName("javax.servlet", "Filter") }
19+
}
20+
21+
class ServletRequestInterface extends Interface {
22+
ServletRequestInterface() { this.hasQualifiedName("javax.servlet.http", "HttpServletRequest") }
23+
}
24+
25+
class GetRequestURIMethodAccess extends MethodAccess {
26+
GetRequestURIMethodAccess() {
27+
this.getMethod().getName() = "getRequestURI" and
28+
this.getMethod().getDeclaringType() instanceof ServletRequestInterface
29+
}
30+
}
31+
32+
private module UnsafeURICheckConfig implements DataFlow::ConfigSig {
33+
predicate isSource(DataFlow::Node source) {
34+
exists(GetRequestURIMethodAccess ma |
35+
ma.getEnclosingCallable().getDeclaringType().getASourceSupertype*() instanceof
36+
ServletFilterInterface and
37+
source.asExpr() = ma
38+
)
39+
}
40+
41+
predicate isSink(DataFlow::Node sink) {
42+
exists(MethodAccess ma |
43+
// java.util.regex.Pattern.matcher("aaaaab");
44+
ma.getMethod().getName() = "matcher" and
45+
ma.getMethod().getDeclaringType().hasQualifiedName("java.util.regex", "Pattern") and
46+
sink.asExpr() = ma.getArgument(0)
47+
or
48+
// java.util.regex.Pattern.matches("a*b", "aaaaab");
49+
ma.getMethod().getName() = "matches" and
50+
ma.getMethod().getDeclaringType().hasQualifiedName("java.util.regex", "Pattern") and
51+
sink.asExpr() = ma.getArgument(1)
52+
or
53+
ma.getMethod().getName() = "matches" and
54+
ma.getMethod().getDeclaringType() instanceof TypeString and
55+
sink.asExpr() = ma.getQualifier()
56+
or
57+
ma.getMethod().getName() = ["startsWith", "endsWith"] and
58+
ma.getMethod().getDeclaringType() instanceof TypeString and
59+
not ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "/" and
60+
sink.asExpr() = ma.getQualifier()
61+
)
62+
}
63+
}
64+
65+
module UnsafeURICheckFlow = TaintTracking::Global<UnsafeURICheckConfig>;
66+
67+
from UnsafeURICheckFlow::PathNode source, UnsafeURICheckFlow::PathNode sink
68+
where UnsafeURICheckFlow::flowPath(source, sink)
69+
select sink.getNode(), source, sink, "Unsafe URI check"

‎java/src/security/CWE-094/GroovyCodeInjection.ql

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/**
2+
* @name Code Injection
3+
* @description Code Injection may allow attackers to
4+
* execute arbitrary code.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id githubsecuritylab/code-injection
9+
* @tags security
10+
* external/cwe/cwe-94
11+
*/
12+
13+
import java
14+
import semmle.code.java.dataflow.FlowSources
15+
import GroovyCodeInjectionFlow::PathGraph
16+
17+
class ParseClassMethod extends Method {
18+
ParseClassMethod() {
19+
this.getDeclaringType()
20+
.getASourceSupertype*()
21+
.hasQualifiedName("groovy.lang", "GroovyClassLoader") and
22+
this.hasName("parseClass") and
23+
(
24+
this.getParameterType(0).(RefType).hasQualifiedName("java.lang", "String") or
25+
this.getParameterType(0).(RefType).hasQualifiedName("java.io", "InputStream") or
26+
this.getParameterType(0).(RefType).hasQualifiedName("java.io", "Reader")
27+
)
28+
or
29+
this.getDeclaringType().getASourceSupertype*().hasQualifiedName("groovy.lang", "GroovyShell") and
30+
(this.hasName("parse") or this.hasName("evaluate")) and
31+
(
32+
this.getParameterType(0).(RefType).hasQualifiedName("java.lang", "String") or
33+
this.getParameterType(0).(RefType).hasQualifiedName("java.io", "Reader")
34+
)
35+
}
36+
}
37+
38+
class GroovyCodeInjectionSink extends DataFlow::ExprNode {
39+
GroovyCodeInjectionSink() {
40+
exists(MethodAccess ma |
41+
ma.getMethod() instanceof ParseClassMethod and
42+
this.getExpr() = ma.getArgument(0)
43+
)
44+
}
45+
}
46+
47+
private module GroovyCodeInjectionConfig implements DataFlow::ConfigSig {
48+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
49+
50+
predicate isSink(DataFlow::Node sink) { sink instanceof GroovyCodeInjectionSink }
51+
}
52+
53+
module GroovyCodeInjectionFlow = TaintTracking::Global<GroovyCodeInjectionConfig>;
54+
55+
from GroovyCodeInjectionFlow::PathNode source, GroovyCodeInjectionFlow::PathNode sink
56+
where GroovyCodeInjectionFlow::flowPath(source, sink)
57+
select sink, source, sink, "Groovy code injection at $@.", sink.getNode(), "user input"

‎java/src/security/CWE-094/RhinoScriptInjection.ql

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/**
2+
* @name Rhino Script Injection
3+
* @kind path-problem
4+
* @problem.severity error
5+
* @precision high
6+
* @id githubsecuritylab/rhino-script-injection
7+
* @tags security
8+
* external/cwe/cwe-094
9+
*/
10+
11+
import java
12+
import semmle.code.java.dataflow.FlowSources
13+
import RhinoInjectionFlow::PathGraph
14+
15+
class RhinoContextType extends Class {
16+
RhinoContextType() { hasQualifiedName("org.mozilla.javascript", "Context") }
17+
}
18+
19+
class CompileMethod extends Method {
20+
CompileMethod() {
21+
this.getDeclaringType() instanceof RhinoContextType and
22+
this.hasName(["compileFunction", "compileReader"])
23+
}
24+
}
25+
26+
class EvaluateMethod extends Method {
27+
EvaluateMethod() {
28+
this.getDeclaringType() instanceof RhinoContextType and
29+
this.hasName(["evaluateString", "evaluateReader"])
30+
}
31+
}
32+
33+
class CompileScriptMethod extends Method {
34+
CompileScriptMethod() {
35+
this.getDeclaringType() instanceof RhinoContextType and
36+
this.hasName("compileScript")
37+
}
38+
}
39+
40+
class RhinoInjectionSink extends DataFlow::ExprNode {
41+
RhinoInjectionSink() {
42+
exists(MethodAccess ma |
43+
(ma.getMethod() instanceof CompileMethod or ma.getMethod() instanceof EvaluateMethod) and
44+
this.getExpr() = ma.getArgument(1)
45+
or
46+
ma.getMethod() instanceof CompileScriptMethod and
47+
this.getExpr() = ma.getArgument(0)
48+
)
49+
}
50+
}
51+
52+
private module RhinoInjectionConfig implements DataFlow::ConfigSig {
53+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
54+
55+
predicate isSink(DataFlow::Node sink) { sink instanceof RhinoInjectionSink }
56+
}
57+
58+
module RhinoInjectionFlow = TaintTracking::Global<RhinoInjectionConfig>;
59+
60+
from RhinoInjectionFlow::PathNode source, RhinoInjectionFlow::PathNode sink
61+
where RhinoInjectionFlow::flowPath(source, sink)
62+
select sink.getNode(), source, sink, "Rhino script injection from $@.", source.getNode(),
63+
"this user input"

0 commit comments

Comments
 (0)