Skip to content

Commit 60fc607

Browse files
haby0haby0
committed
Modify ql
1 parent 12f47bc commit 60fc607

File tree

9 files changed

+138
-118
lines changed

9 files changed

+138
-118
lines changed

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

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -24,34 +24,34 @@ class UnsafeDeserializationConfig extends TaintTracking::Configuration {
2424

2525
override predicate isAdditionalTaintStep(DataFlow::Node prod, DataFlow::Node succ) {
2626
exists(ClassInstanceExpr cie |
27-
cie.getConstructor().getDeclaringType() instanceof JsonReader and
2827
cie.getArgument(0) = prod.asExpr() and
2928
cie = succ.asExpr() and
30-
not exists(SafeJsonIo sji | sji.hasFlowToExpr(cie.getArgument(1)))
29+
(
30+
cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader or
31+
cie.getConstructor().getDeclaringType() instanceof YamlBeansReader or
32+
cie.getConstructor().getDeclaringType().getASupertype*() instanceof UnsafeHessianInput or
33+
cie.getConstructor().getDeclaringType() instanceof BurlapInput
34+
)
3135
)
3236
or
33-
exists(ClassInstanceExpr cie |
34-
cie.getConstructor().getDeclaringType() instanceof YamlReader and
35-
cie.getArgument(0) = prod.asExpr() and
36-
cie = succ.asExpr()
37-
)
38-
or
39-
exists(ClassInstanceExpr cie |
40-
cie.getConstructor().getDeclaringType() instanceof UnSafeHessianInput and
41-
cie.getArgument(0) = prod.asExpr() and
42-
cie = succ.asExpr()
37+
exists(MethodAccess ma |
38+
ma.getMethod() instanceof BurlapInputInitMethod and
39+
ma.getArgument(0) = prod.asExpr() and
40+
ma.getQualifier() = succ.asExpr()
4341
)
44-
or
42+
}
43+
44+
override predicate isSanitizer(DataFlow::Node node) {
4545
exists(ClassInstanceExpr cie |
46-
cie.getConstructor().getDeclaringType() instanceof BurlapInput and
47-
cie.getArgument(0) = prod.asExpr() and
48-
cie = succ.asExpr()
46+
cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader and
47+
cie = node.asExpr() and
48+
exists(SafeJsonIoConfig sji | sji.hasFlowToExpr(cie.getArgument(1)))
4949
)
5050
or
5151
exists(MethodAccess ma |
52-
ma.getMethod() instanceof BurlapInputInitMethod and
53-
ma.getArgument(0) = prod.asExpr() and
54-
ma.getQualifier() = succ.asExpr()
52+
ma.getMethod() instanceof JsonIoJsonToJavaMethod and
53+
ma.getArgument(0) = node.asExpr() and
54+
exists(SafeJsonIoConfig sji | sji.hasFlowToExpr(ma.getArgument(1)))
5555
)
5656
}
5757
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ import java
77
/**
88
* The class `org.exolab.castor.xml.Unmarshaller`.
99
*/
10-
class Unmarshaller extends RefType {
11-
Unmarshaller() { this.hasQualifiedName("org.exolab.castor.xml", "Unmarshaller") }
10+
class CastorUnmarshaller extends RefType {
11+
CastorUnmarshaller() { this.hasQualifiedName("org.exolab.castor.xml", "Unmarshaller") }
1212
}
1313

1414
/** A method with the name `unmarshal` declared in `org.exolab.castor.xml.Unmarshaller`. */
1515
class UnmarshalMethod extends Method {
1616
UnmarshalMethod() {
17-
this.getDeclaringType() instanceof Unmarshaller and
17+
this.getDeclaringType() instanceof CastorUnmarshaller and
1818
this.getName() = "unmarshal"
1919
}
2020
}

‎java/ql/src/semmle/code/java/frameworks/Hessian.qll renamed to ‎java/ql/src/semmle/code/java/frameworks/HessianBurlap.qll

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,26 @@
11
/**
2-
* Provides classes and predicates for working with the Hession framework.
2+
* Provides classes and predicates for working with the HessianBurlap framework.
33
*/
44

55
import java
66

77
/**
8-
* The class `com.caucho.hessian.io.HessianInput` or `com.caucho.hessian.io.Hessian2Input`.
8+
* The class `com.caucho.hessian.io.AbstractHessianInput` or `com.alibaba.com.caucho.hessian.io.Hessian2StreamingInput`.
99
*/
10-
class UnSafeHessianInput extends RefType {
11-
UnSafeHessianInput() {
12-
this.hasQualifiedName("com.caucho.hessian.io", ["HessianInput", "Hessian2Input"])
10+
class UnsafeHessianInput extends RefType {
11+
UnsafeHessianInput() {
12+
this.hasQualifiedName(["com.caucho.hessian.io", "com.alibaba.com.caucho.hessian.io"],
13+
["AbstractHessianInput", "Hessian2StreamingInput"])
1314
}
1415
}
1516

1617
/**
17-
* A HessianInput readObject method. This is either `HessianInput.readObject` or `Hessian2Input.readObject`.
18+
* A AbstractHessianInput or Hessian2StreamingInput subclass readObject method.
19+
* This is either `AbstractHessianInput.readObject` or `Hessian2StreamingInput.readObject`.
1820
*/
19-
class UnSafeHessianInputReadObjectMethod extends Method {
20-
UnSafeHessianInputReadObjectMethod() {
21-
this.getDeclaringType() instanceof UnSafeHessianInput and
21+
class UnsafeHessianInputReadObjectMethod extends Method {
22+
UnsafeHessianInputReadObjectMethod() {
23+
this.getDeclaringType().getASupertype*() instanceof UnsafeHessianInput and
2224
this.getName() = "readObject"
2325
}
2426
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ class JYaml extends RefType {
1515
* A JYaml unsafe load method. This is either `YAML.load` or
1616
* `YAML.loadType` or `YAML.loadStream` or `YAML.loadStreamOfType`.
1717
*/
18-
class JYamlUnSafeLoadMethod extends Method {
19-
JYamlUnSafeLoadMethod() {
18+
class JYamlUnsafeLoadMethod extends Method {
19+
JYamlUnsafeLoadMethod() {
2020
this.getDeclaringType() instanceof JYaml and
2121
this.getName() in ["load", "loadType", "loadStream", "loadStreamOfType"]
2222
}
@@ -33,8 +33,8 @@ class JYamlConfig extends RefType {
3333
* A JYamlConfig unsafe load method. This is either `YamlConfig.load` or
3434
* `YAML.loadType` or `YamlConfig.loadStream` or `YamlConfig.loadStreamOfType`.
3535
*/
36-
class JYamlConfigUnSafeLoadMethod extends Method {
37-
JYamlConfigUnSafeLoadMethod() {
36+
class JYamlConfigUnsafeLoadMethod extends Method {
37+
JYamlConfigUnsafeLoadMethod() {
3838
this.getDeclaringType() instanceof JYamlConfig and
3939
this.getName() in ["load", "loadType", "loadStream", "loadStreamOfType"]
4040
}

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

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,32 +4,34 @@
44

55
import java
66
import semmle.code.java.Maps
7+
import semmle.code.java.dataflow.DataFlow
8+
import semmle.code.java.dataflow.DataFlow2
79

810
/**
911
* The class `com.cedarsoftware.util.io.JsonReader`.
1012
*/
11-
class JsonReader extends RefType {
12-
JsonReader() { this.hasQualifiedName("com.cedarsoftware.util.io", "JsonReader") }
13+
class JsonIoJsonReader extends RefType {
14+
JsonIoJsonReader() { this.hasQualifiedName("com.cedarsoftware.util.io", "JsonReader") }
1315
}
1416

1517
/** A method with the name `jsonToJava` declared in `com.cedarsoftware.util.io.JsonReader`. */
1618
class JsonIoJsonToJavaMethod extends Method {
1719
JsonIoJsonToJavaMethod() {
18-
this.getDeclaringType() instanceof JsonReader and
20+
this.getDeclaringType() instanceof JsonIoJsonReader and
1921
this.getName() = "jsonToJava"
2022
}
2123
}
2224

2325
/** A method with the name `readObject` declared in `com.cedarsoftware.util.io.JsonReader`. */
2426
class JsonIoReadObjectMethod extends Method {
2527
JsonIoReadObjectMethod() {
26-
this.getDeclaringType() instanceof JsonReader and
28+
this.getDeclaringType() instanceof JsonIoJsonReader and
2729
this.getName() = "readObject"
2830
}
2931
}
3032

3133
/**
32-
* A call to `Map.put` method, set the value of the `USE_MAPS` key to `true`.
34+
* A call to `Map.put` method, set the value of the `USE_MAPS` key to `true`.
3335
*/
3436
class JsonIoSafeOptionalArgs extends MethodAccess {
3537
JsonIoSafeOptionalArgs() {
@@ -39,3 +41,27 @@ class JsonIoSafeOptionalArgs extends MethodAccess {
3941
this.getArgument(1).(CompileTimeConstantExpr).getBooleanValue() = true
4042
}
4143
}
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 JsonIoSafeOptionalArgs 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: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ import java
77
/**
88
* The class `com.esotericsoftware.yamlbeans.YamlReader`.
99
*/
10-
class YamlReader extends RefType {
11-
YamlReader() { this.hasQualifiedName("com.esotericsoftware.yamlbeans", "YamlReader") }
10+
class YamlBeansReader extends RefType {
11+
YamlBeansReader() { this.hasQualifiedName("com.esotericsoftware.yamlbeans", "YamlReader") }
1212
}
1313

1414
/** A method with the name `read` declared in `com.esotericsoftware.yamlbeans.YamlReader`. */
15-
class YamlReaderReadMethod extends Method {
16-
YamlReaderReadMethod() {
17-
this.getDeclaringType() instanceof YamlReader and
15+
class YamlBeansReaderReadMethod extends Method {
16+
YamlBeansReaderReadMethod() {
17+
this.getDeclaringType() instanceof YamlBeansReader and
1818
this.getName() = "read"
1919
}
2020
}

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

Lines changed: 6 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import semmle.code.java.frameworks.FastJson
55
import semmle.code.java.frameworks.JYaml
66
import semmle.code.java.frameworks.JsonIo
77
import semmle.code.java.frameworks.YamlBeans
8-
import semmle.code.java.frameworks.Hessian
8+
import semmle.code.java.frameworks.HessianBurlap
99
import semmle.code.java.frameworks.Castor
1010
import semmle.code.java.frameworks.apache.Lang
1111

@@ -55,29 +55,6 @@ class SafeKryo extends DataFlow2::Configuration {
5555
}
5656
}
5757

58-
class SafeJsonIo extends DataFlow2::Configuration {
59-
SafeJsonIo() { this = "UnsafeDeserialization::SafeJsonIo" }
60-
61-
override predicate isSource(DataFlow::Node src) {
62-
exists(MethodAccess ma |
63-
ma instanceof JsonIoSafeOptionalArgs and
64-
src.asExpr() = ma.getQualifier()
65-
)
66-
}
67-
68-
override predicate isSink(DataFlow::Node sink) {
69-
exists(MethodAccess ma |
70-
ma.getMethod() instanceof JsonIoJsonToJavaMethod and
71-
sink.asExpr() = ma.getArgument(1)
72-
)
73-
or
74-
exists(ClassInstanceExpr cie |
75-
cie.getConstructor().getDeclaringType() instanceof JsonReader and
76-
sink.asExpr() = cie.getArgument(1)
77-
)
78-
}
79-
}
80-
8158
predicate unsafeDeserialization(MethodAccess ma, Expr sink) {
8259
exists(Method m | m = ma.getMethod() |
8360
m instanceof ObjectInputStreamReadObjectMethod and
@@ -110,22 +87,21 @@ predicate unsafeDeserialization(MethodAccess ma, Expr sink) {
11087
not fastJsonLooksSafe() and
11188
sink = ma.getArgument(0)
11289
or
113-
ma.getMethod() instanceof JYamlUnSafeLoadMethod and
90+
ma.getMethod() instanceof JYamlUnsafeLoadMethod and
11491
sink = ma.getArgument(0)
11592
or
116-
ma.getMethod() instanceof JYamlConfigUnSafeLoadMethod and
93+
ma.getMethod() instanceof JYamlConfigUnsafeLoadMethod and
11794
sink = ma.getArgument(0)
11895
or
11996
ma.getMethod() instanceof JsonIoJsonToJavaMethod and
120-
sink = ma.getArgument(0) and
121-
not exists(SafeJsonIo sji | sji.hasFlowToExpr(ma.getArgument(1)))
97+
sink = ma.getArgument(0)
12298
or
12399
ma.getMethod() instanceof JsonIoReadObjectMethod and
124100
sink = ma.getQualifier()
125101
or
126-
ma.getMethod() instanceof YamlReaderReadMethod and sink = ma.getQualifier()
102+
ma.getMethod() instanceof YamlBeansReaderReadMethod and sink = ma.getQualifier()
127103
or
128-
ma.getMethod() instanceof UnSafeHessianInputReadObjectMethod and sink = ma.getQualifier()
104+
ma.getMethod() instanceof UnsafeHessianInputReadObjectMethod and sink = ma.getQualifier()
129105
or
130106
ma.getMethod() instanceof UnmarshalMethod and sink = ma.getAnArgument()
131107
or

‎java/ql/test/query-tests/security/CWE-502/C.java

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,8 @@ public void bad2(HttpServletRequest request) {
4242

4343
JsonReader.jsonToJava(data); //bad
4444

45-
JsonReader.jsonToJava(data, hashMap); //good
46-
4745
JsonReader jr = new JsonReader(data, null); //bad
4846
jr.readObject();
49-
50-
JsonReader jr1 = new JsonReader(data, hashMap); //good
51-
jr1.readObject();
5247
}
5348

5449
@GetMapping(value = "yamlbeans")
@@ -95,4 +90,25 @@ public void bad7(HttpServletRequest request) throws Exception {
9590
burlapInput1.init(is);
9691
burlapInput1.readObject(); //bad
9792
}
93+
94+
@GetMapping(value = "jsonio1")
95+
public void good1(HttpServletRequest request) {
96+
String data = request.getParameter("data");
97+
98+
HashMap hashMap = new HashMap();
99+
hashMap.put("USE_MAPS", true);
100+
101+
JsonReader.jsonToJava(data, hashMap); //good
102+
}
103+
104+
@GetMapping(value = "jsonio2")
105+
public void good2(HttpServletRequest request) {
106+
String data = request.getParameter("data");
107+
108+
HashMap hashMap = new HashMap();
109+
hashMap.put("USE_MAPS", true);
110+
111+
JsonReader jr1 = new JsonReader(data, hashMap); //good
112+
jr1.readObject();
113+
}
98114
}

0 commit comments

Comments
 (0)