Skip to content

Commit 6ca8d69

Browse files
aschackmullaschackmull
authored
Merge pull request github#5881 from haby0/java/UnsafeDeserialization
Java: CWE-502 Add UnsafeDeserialization sinks
2 parents 8fe2f4a + 363ad5b commit 6ca8d69

39 files changed

+2073
-3
lines changed

‎java/change-notes/2021-05-17-add-unsafe-deserialization-sinks.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
lgtm,codescanning
2+
* The "Deserialization of user-controlled data" (`java/unsafe-deserialization`) query
3+
now recognizes `JYaml`, `JsonIO`, `YAMLBeans`, `Castor`, `Hessian` and `Burlap` deserialization.

‎java/ql/src/Security/CWE/CWE-502/UnsafeDeserialization.qhelp

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ may have unforeseen effects, such as the execution of arbitrary code.
1414
</p>
1515
<p>
1616
There are many different serialization frameworks. This query currently
17-
supports Kryo, XmlDecoder, XStream, SnakeYaml, and Java IO serialization through
18-
<code>ObjectInputStream</code>/<code>ObjectOutputStream</code>.
17+
supports Kryo, XmlDecoder, XStream, SnakeYaml, JYaml, JsonIO, YAMLBeans, HessianBurlap, Castor, Burlap
18+
and Java IO serialization through <code>ObjectInputStream</code>/<code>ObjectOutputStream</code>.
1919
</p>
2020
</overview>
2121

@@ -75,6 +75,22 @@ Alvaro Muñoz &amp; Christian Schneider, RSAConference 2016:
7575
SnakeYaml documentation on deserialization:
7676
<a href="https://bitbucket.org/asomov/snakeyaml/wiki/Documentation#markdown-header-loading-yaml">SnakeYaml deserialization</a>.
7777
</li>
78+
<li>
79+
Hessian deserialization and related gadget chains:
80+
<a href="https://paper.seebug.org/1137/">Hessian deserialization</a>.
81+
</li>
82+
<li>
83+
Castor and Hessian java deserialization vulnerabilities:
84+
<a href="https://securitylab.github.com/research/hessian-java-deserialization-castor-vulnerabilities/">Castor and Hessian deserialization</a>.
85+
</li>
86+
<li>
87+
Remote code execution in JYaml library:
88+
<a href="https://www.cybersecurity-help.cz/vdb/SB2020022512">JYaml deserialization</a>.
89+
</li>
90+
<li>
91+
JsonIO deserialization vulnerabilities:
92+
<a href="https://klezvirus.github.io/Advanced-Web-Hacking/Serialisation/">JsonIO deserialization</a>.
93+
</li>
7894
</references>
7995

8096
</qhelp>

‎java/ql/src/Security/CWE/CWE-502/UnsafeDeserialization.ql

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,39 @@ class UnsafeDeserializationConfig extends TaintTracking::Configuration {
2222
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
2323

2424
override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeDeserializationSink }
25+
26+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
27+
exists(ClassInstanceExpr cie |
28+
cie.getArgument(0) = pred.asExpr() and
29+
cie = succ.asExpr() and
30+
(
31+
cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader or
32+
cie.getConstructor().getDeclaringType() instanceof YamlBeansReader or
33+
cie.getConstructor().getDeclaringType().getASupertype*() instanceof UnsafeHessianInput or
34+
cie.getConstructor().getDeclaringType() instanceof BurlapInput
35+
)
36+
)
37+
or
38+
exists(MethodAccess ma |
39+
ma.getMethod() instanceof BurlapInputInitMethod and
40+
ma.getArgument(0) = pred.asExpr() and
41+
ma.getQualifier() = succ.asExpr()
42+
)
43+
}
44+
45+
override predicate isSanitizer(DataFlow::Node node) {
46+
exists(ClassInstanceExpr cie |
47+
cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader and
48+
cie = node.asExpr() and
49+
exists(SafeJsonIoConfig sji | sji.hasFlowToExpr(cie.getArgument(1)))
50+
)
51+
or
52+
exists(MethodAccess ma |
53+
ma.getMethod() instanceof JsonIoJsonToJavaMethod and
54+
ma.getArgument(0) = node.asExpr() and
55+
exists(SafeJsonIoConfig sji | sji.hasFlowToExpr(ma.getArgument(1)))
56+
)
57+
}
2558
}
2659

2760
from DataFlow::PathNode source, DataFlow::PathNode sink, UnsafeDeserializationConfig conf

‎java/ql/src/semmle/code/java/frameworks/Castor.qll

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* Provides classes and predicates for working with the Castor framework.
3+
*/
4+
5+
import java
6+
7+
/**
8+
* The class `org.exolab.castor.xml.Unmarshaller`.
9+
*/
10+
class CastorUnmarshaller extends RefType {
11+
CastorUnmarshaller() { this.hasQualifiedName("org.exolab.castor.xml", "Unmarshaller") }
12+
}
13+
14+
/** A method with the name `unmarshal` declared in `org.exolab.castor.xml.Unmarshaller`. */
15+
class CastorUnmarshalMethod extends Method {
16+
CastorUnmarshalMethod() {
17+
this.getDeclaringType() instanceof CastorUnmarshaller and
18+
this.getName() = "unmarshal"
19+
}
20+
}

‎java/ql/src/semmle/code/java/frameworks/HessianBurlap.qll

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/**
2+
* Provides classes and predicates for working with the HessianBurlap framework.
3+
*/
4+
5+
import java
6+
7+
/**
8+
* The classes `[com.alibaba.]com.caucho.hessian.io.AbstractHessianInput` or `[com.alibaba.]com.caucho.hessian.io.Hessian2StreamingInput`.
9+
*/
10+
class UnsafeHessianInput extends RefType {
11+
UnsafeHessianInput() {
12+
this.hasQualifiedName(["com.caucho.hessian.io", "com.alibaba.com.caucho.hessian.io"],
13+
["AbstractHessianInput", "Hessian2StreamingInput"])
14+
}
15+
}
16+
17+
/**
18+
* A AbstractHessianInput or Hessian2StreamingInput subclass readObject method.
19+
* This is either `AbstractHessianInput.readObject` or `Hessian2StreamingInput.readObject`.
20+
*/
21+
class UnsafeHessianInputReadObjectMethod extends Method {
22+
UnsafeHessianInputReadObjectMethod() {
23+
this.getDeclaringType().getASupertype*() instanceof UnsafeHessianInput and
24+
this.getName() = "readObject"
25+
}
26+
}
27+
28+
/**
29+
* The class `com.caucho.burlap.io.BurlapInput`.
30+
*/
31+
class BurlapInput extends RefType {
32+
BurlapInput() { this.hasQualifiedName("com.caucho.burlap.io", "BurlapInput") }
33+
}
34+
35+
/** A method with the name `readObject` declared in `com.caucho.burlap.io.BurlapInput`. */
36+
class BurlapInputReadObjectMethod extends Method {
37+
BurlapInputReadObjectMethod() {
38+
this.getDeclaringType() instanceof BurlapInput and
39+
this.getName() = "readObject"
40+
}
41+
}
42+
43+
/** A method with the name `init` declared in `com.caucho.burlap.io.BurlapInput`. */
44+
class BurlapInputInitMethod extends Method {
45+
BurlapInputInitMethod() {
46+
this.getDeclaringType() instanceof BurlapInput and
47+
this.getName() = "init"
48+
}
49+
}

‎java/ql/src/semmle/code/java/frameworks/JYaml.qll

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/**
2+
* Provides classes and predicates for working with the JYaml framework.
3+
*/
4+
5+
import java
6+
7+
/**
8+
* The class `org.ho.yaml.Yaml` or `org.ho.yaml.YamlConfig`.
9+
*/
10+
class JYamlLoader extends RefType {
11+
JYamlLoader() { this.hasQualifiedName("org.ho.yaml", ["Yaml", "YamlConfig"]) }
12+
}
13+
14+
/**
15+
* A JYaml unsafe load method, declared on either `Yaml` or `YamlConfig`.
16+
*/
17+
class JYamlLoaderUnsafeLoadMethod extends Method {
18+
JYamlLoaderUnsafeLoadMethod() {
19+
this.getDeclaringType() instanceof JYamlLoader and
20+
this.getName() in ["load", "loadType", "loadStream", "loadStreamOfType"]
21+
}
22+
}

‎java/ql/src/semmle/code/java/frameworks/JsonIo.qll

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/**
2+
* Provides classes and predicates for working with the Json-io framework.
3+
*/
4+
5+
import java
6+
import semmle.code.java.Maps
7+
import semmle.code.java.dataflow.DataFlow
8+
import semmle.code.java.dataflow.DataFlow2
9+
10+
/**
11+
* The class `com.cedarsoftware.util.io.JsonReader`.
12+
*/
13+
class JsonIoJsonReader extends RefType {
14+
JsonIoJsonReader() { this.hasQualifiedName("com.cedarsoftware.util.io", "JsonReader") }
15+
}
16+
17+
/** A method with the name `jsonToJava` declared in `com.cedarsoftware.util.io.JsonReader`. */
18+
class JsonIoJsonToJavaMethod extends Method {
19+
JsonIoJsonToJavaMethod() {
20+
this.getDeclaringType() instanceof JsonIoJsonReader and
21+
this.getName() = "jsonToJava"
22+
}
23+
}
24+
25+
/** A method with the name `readObject` declared in `com.cedarsoftware.util.io.JsonReader`. */
26+
class JsonIoReadObjectMethod extends Method {
27+
JsonIoReadObjectMethod() {
28+
this.getDeclaringType() instanceof JsonIoJsonReader and
29+
this.getName() = "readObject"
30+
}
31+
}
32+
33+
/**
34+
* A call to `Map.put` method, set the value of the `USE_MAPS` key to `true`.
35+
*/
36+
class JsonIoUseMapsSetter extends MethodAccess {
37+
JsonIoUseMapsSetter() {
38+
this.getMethod().getDeclaringType().getASourceSupertype*() instanceof MapType and
39+
this.getMethod().hasName("put") and
40+
this.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "USE_MAPS" and
41+
this.getArgument(1).(CompileTimeConstantExpr).getBooleanValue() = true
42+
}
43+
}
44+
45+
/** A data flow configuration tracing flow from JsonIo safe settings. */
46+
class SafeJsonIoConfig extends DataFlow2::Configuration {
47+
SafeJsonIoConfig() { this = "UnsafeDeserialization::SafeJsonIoConfig" }
48+
49+
override predicate isSource(DataFlow::Node src) {
50+
exists(MethodAccess ma |
51+
ma instanceof JsonIoUseMapsSetter and
52+
src.asExpr() = ma.getQualifier()
53+
)
54+
}
55+
56+
override predicate isSink(DataFlow::Node sink) {
57+
exists(MethodAccess ma |
58+
ma.getMethod() instanceof JsonIoJsonToJavaMethod and
59+
sink.asExpr() = ma.getArgument(1)
60+
)
61+
or
62+
exists(ClassInstanceExpr cie |
63+
cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader and
64+
sink.asExpr() = cie.getArgument(1)
65+
)
66+
}
67+
}

‎java/ql/src/semmle/code/java/frameworks/YamlBeans.qll

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* Provides classes and predicates for working with the YamlBeans framework.
3+
*/
4+
5+
import java
6+
7+
/**
8+
* The class `com.esotericsoftware.yamlbeans.YamlReader`.
9+
*/
10+
class YamlBeansReader extends RefType {
11+
YamlBeansReader() { this.hasQualifiedName("com.esotericsoftware.yamlbeans", "YamlReader") }
12+
}
13+
14+
/** A method with the name `read` declared in `com.esotericsoftware.yamlbeans.YamlReader`. */
15+
class YamlBeansReaderReadMethod extends Method {
16+
YamlBeansReaderReadMethod() {
17+
this.getDeclaringType() instanceof YamlBeansReader and
18+
this.getName() = "read"
19+
}
20+
}

‎java/ql/src/semmle/code/java/security/UnsafeDeserialization.qll

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@ import semmle.code.java.frameworks.Kryo
22
import semmle.code.java.frameworks.XStream
33
import semmle.code.java.frameworks.SnakeYaml
44
import semmle.code.java.frameworks.FastJson
5+
import semmle.code.java.frameworks.JYaml
6+
import semmle.code.java.frameworks.JsonIo
7+
import semmle.code.java.frameworks.YamlBeans
8+
import semmle.code.java.frameworks.HessianBurlap
9+
import semmle.code.java.frameworks.Castor
510
import semmle.code.java.frameworks.apache.Lang
611

712
class ObjectInputStreamReadObjectMethod extends Method {
@@ -140,6 +145,23 @@ predicate unsafeDeserialization(MethodAccess ma, Expr sink) {
140145
ma.getMethod() instanceof FastJsonParseMethod and
141146
not fastJsonLooksSafe() and
142147
sink = ma.getArgument(0)
148+
or
149+
ma.getMethod() instanceof JYamlLoaderUnsafeLoadMethod and
150+
sink = ma.getArgument(0)
151+
or
152+
ma.getMethod() instanceof JsonIoJsonToJavaMethod and
153+
sink = ma.getArgument(0)
154+
or
155+
ma.getMethod() instanceof JsonIoReadObjectMethod and
156+
sink = ma.getQualifier()
157+
or
158+
ma.getMethod() instanceof YamlBeansReaderReadMethod and sink = ma.getQualifier()
159+
or
160+
ma.getMethod() instanceof UnsafeHessianInputReadObjectMethod and sink = ma.getQualifier()
161+
or
162+
ma.getMethod() instanceof CastorUnmarshalMethod and sink = ma.getAnArgument()
163+
or
164+
ma.getMethod() instanceof BurlapInputReadObjectMethod and sink = ma.getQualifier()
143165
)
144166
}
145167

0 commit comments

Comments
 (0)