Skip to content

Commit 62c0eea

Browse files
semmle-qlcisemmle-qlci
authored
Merge pull request #939 from yh-semmle/java-frameworks
Approved by pavgust
2 parents 1b25573 + ca3aaa8 commit 62c0eea

File tree

13 files changed

+316
-0
lines changed

13 files changed

+316
-0
lines changed

‎change-notes/1.20/analysis-java.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,5 +28,9 @@
2828
* Taint tracking now includes additional default data-flow steps through
2929
collections, maps, and iterators. This affects all security queries, which
3030
can report more results based on such paths.
31+
* The `FlowSources` and `TaintTracking` libraries are extended to cover additional remote user
32+
input and taint steps from the Apache Thrift, Apache Struts, Guice and Protobuf frameworks.
33+
This affects all security queries, which may yield additional results on projects
34+
that use these frameworks.
3135

3236

‎java/ql/src/semmle/code/java/dataflow/FlowSources.qll‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ import semmle.code.java.frameworks.android.WebView
1717
import semmle.code.java.frameworks.JaxWS
1818
import semmle.code.java.frameworks.android.Intent
1919
import semmle.code.java.frameworks.SpringWeb
20+
import semmle.code.java.frameworks.Guice
21+
import semmle.code.java.frameworks.struts.StrutsActions
22+
import semmle.code.java.frameworks.Thrift
2023

2124
/** Class for `tainted` user input. */
2225
abstract class UserInput extends DataFlow::Node { }
@@ -69,6 +72,15 @@ class RemoteUserInput extends UserInput {
6972
)
7073
or
7174
this.asParameter().getAnAnnotation() instanceof SpringServletInputAnnotation
75+
or
76+
exists(GuiceRequestParametersAnnotation a |
77+
a = this.asParameter().getAnAnnotation() or
78+
a = this.asExpr().(FieldRead).getField().getAnAnnotation()
79+
)
80+
or
81+
exists(Struts2ActionSupportClass c | c.getASetterMethod().getField() = this.asExpr().(FieldRead).getField())
82+
or
83+
exists(ThriftIface i | i.getAnImplementingMethod().getAParameter() = this.asParameter())
7284
}
7385

7486
/**

‎java/ql/src/semmle/code/java/dataflow/TaintTracking.qll‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ private import DefUse
1212
private import semmle.code.java.security.SecurityTests
1313
private import semmle.code.java.security.Validation
1414
private import semmle.code.java.frameworks.android.Intent
15+
private import semmle.code.java.frameworks.Guice
16+
private import semmle.code.java.frameworks.Protobuf
1517
private import semmle.code.java.Maps
1618

1719
module TaintTracking {
@@ -471,6 +473,10 @@ module TaintTracking {
471473
or
472474
m.getDeclaringType().hasQualifiedName("java.nio", "ByteBuffer") and
473475
m.hasName("get")
476+
or
477+
m = any(GuiceProvider gp).getAnOverridingGetMethod()
478+
or
479+
m = any(ProtobufMessageLite p).getAGetterMethod()
474480
}
475481

476482
private class StringReplaceMethod extends Method {
@@ -575,6 +581,12 @@ module TaintTracking {
575581
method.getDeclaringType().hasQualifiedName("javax.xml.transform.sax", "SAXSource") and
576582
method.hasName("sourceToInputSource") and
577583
arg = 0
584+
or
585+
exists(ProtobufParser p | method = p.getAParseFromMethod()) and
586+
arg = 0
587+
or
588+
exists(ProtobufMessageLite m | method = m.getAParseFromMethod()) and
589+
arg = 0
578590
}
579591

580592
/**

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/**
2+
* Provides classes and predicates for working with the Guice framework.
3+
*/
4+
5+
import java
6+
7+
/**
8+
* A `@com.google.inject.servlet.RequestParameters` annotation.
9+
*/
10+
class GuiceRequestParametersAnnotation extends Annotation {
11+
GuiceRequestParametersAnnotation() {
12+
this.getType().hasQualifiedName("com.google.inject.servlet", "RequestParameters")
13+
}
14+
}
15+
16+
/**
17+
* The interface `com.google.inject.Provider`.
18+
*/
19+
class GuiceProvider extends Interface {
20+
GuiceProvider() { this.hasQualifiedName("com.google.inject", "Provider") }
21+
22+
/**
23+
* The method named `get` declared on the interface `com.google.inject.Provider`.
24+
*/
25+
Method getGetMethod() {
26+
result.getDeclaringType() = this and result.getName() = "get" and result.hasNoParameters()
27+
}
28+
29+
/**
30+
* A method that overrides the `get` method on the interface `com.google.inject.Provider`.
31+
*/
32+
Method getAnOverridingGetMethod() {
33+
exists(Method m | m.getSourceDeclaration() = getGetMethod() | result.overrides*(m))
34+
}
35+
}

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/**
2+
* Provides classes and predicates for working with the Protobuf framework.
3+
*/
4+
5+
import java
6+
7+
/**
8+
* The interface `com.google.protobuf.Parser`.
9+
*/
10+
class ProtobufParser extends Interface {
11+
ProtobufParser() { this.hasQualifiedName("com.google.protobuf", "Parser") }
12+
13+
/**
14+
* Gets a method named `parseFrom` (or similar) declared on a subtype of `com.google.protobuf.Parser`.
15+
*/
16+
Method getAParseFromMethod() {
17+
result.getDeclaringType().getASupertype*().getSourceDeclaration() = this and
18+
result.getName().matches("parse%From")
19+
}
20+
}
21+
22+
/**
23+
* The interface `com.google.protobuf.MessageLite`.
24+
*/
25+
class ProtobufMessageLite extends Interface {
26+
ProtobufMessageLite() { this.hasQualifiedName("com.google.protobuf", "MessageLite") }
27+
28+
/**
29+
* Gets a static method named `parseFrom` (or similar) declared on a subtype of the `MessageLite` interface.
30+
*/
31+
Method getAParseFromMethod() {
32+
result = getASubtype+().getAMethod() and
33+
result.getName().matches("parse%From") and
34+
result.isStatic()
35+
}
36+
37+
/**
38+
* Gets a getter method declared on a subtype of the `MessageLite` interface.
39+
*/
40+
Method getAGetterMethod() {
41+
exists(RefType decl | decl = result.getDeclaringType() and decl = this.getASubtype+() |
42+
exists(string name, string suffix |
43+
suffix = "" or
44+
suffix = "list" or
45+
suffix = "map" or
46+
suffix = "ordefault" or
47+
suffix = "orthrow"
48+
|
49+
exists(Field f | f.getDeclaringType() = decl |
50+
f.getName().toLowerCase().replaceAll("_", "") = name
51+
) and
52+
result.getName().toLowerCase() = "get" + name + suffix
53+
)
54+
)
55+
}
56+
}

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* Provides classes and predicates for working with the Apache Thrift framework.
3+
*/
4+
5+
import java
6+
7+
/**
8+
* A file detected as generated by the Apache Thrift Compiler.
9+
*/
10+
class ThriftGeneratedFile extends GeneratedFile {
11+
ThriftGeneratedFile() {
12+
exists(JavadocElement t | t.getFile() = this |
13+
exists(string msg | msg = t.getText() | msg.regexpMatch("(?i).*\\bAutogenerated by Thrift.*"))
14+
)
15+
}
16+
}
17+
18+
/**
19+
* A Thrift `Iface` interface in a class generated by the Apache Thrift Compiler.
20+
*/
21+
class ThriftIface extends Interface {
22+
ThriftIface() {
23+
this.hasName("Iface") and
24+
this.getEnclosingType() instanceof TopLevelType and
25+
this.getFile() instanceof ThriftGeneratedFile
26+
}
27+
28+
Method getAnImplementingMethod() {
29+
result.getDeclaringType().(Class).getASupertype+() = this and
30+
result.overrides(getAMethod()) and
31+
not result.getFile() = this.getFile()
32+
}
33+
}

‎java/ql/src/semmle/code/java/frameworks/struts/StrutsActions.qll‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,3 +124,24 @@ class Struts2PrepareMethod extends Method {
124124
exists(Struts2ActionClass actionClass | this = actionClass.getPrepareMethod())
125125
}
126126
}
127+
128+
/**
129+
* A subclass of the Struts 2 `ActionSupport` class.
130+
*/
131+
class Struts2ActionSupportClass extends Class {
132+
Struts2ActionSupportClass() {
133+
this.getASupertype+().hasQualifiedName("com.opensymphony.xwork2", "ActionSupport")
134+
}
135+
136+
/**
137+
* Gets a setter method declared on a subclass of `ActionSupport`.
138+
*/
139+
SetterMethod getASetterMethod() {
140+
result.getDeclaringType() = this and
141+
result.isPublic() and
142+
exists(string name | result.getField().getName().toLowerCase() = name |
143+
result.getName().toLowerCase().substring(3, result.getName().length()) = name and
144+
result.getName().matches("set%")
145+
)
146+
}
147+
}

‎java/ql/test/library-tests/frameworks/guice/GuiceRequestParameters.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import java.util.Map;
2+
3+
import com.google.inject.Provider;
4+
import com.google.inject.servlet.RequestParameters;
5+
6+
public class GuiceRequestParameters {
7+
@RequestParameters
8+
private Map<String,String> paramMap;
9+
@RequestParameters
10+
private Provider<Map<String,String>> providerMap;
11+
12+
void test(String key) {
13+
String s = paramMap.get(key);
14+
sink(s);
15+
String value = providerMap.get().get(key);
16+
sink(value);
17+
}
18+
19+
private void sink(String s) {}
20+
}

‎java/ql/test/library-tests/frameworks/guice/flow.expected‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
| GuiceRequestParameters.java:13:14:13:21 | paramMap | GuiceRequestParameters.java:14:8:14:8 | s |
2+
| GuiceRequestParameters.java:15:18:15:28 | providerMap | GuiceRequestParameters.java:16:8:16:12 | value |

‎java/ql/test/library-tests/frameworks/guice/flow.ql‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import java
2+
import semmle.code.java.dataflow.FlowSources
3+
import semmle.code.java.dataflow.TaintTracking
4+
5+
class Conf extends TaintTracking::Configuration {
6+
Conf() { this = "conf" }
7+
8+
override predicate isSource(DataFlow::Node src) {
9+
src instanceof RemoteUserInput
10+
}
11+
12+
override predicate isSink(DataFlow::Node sink) {
13+
exists(MethodAccess ma |
14+
sink.asExpr() = ma.getAnArgument() and
15+
ma.getMethod().hasName("sink")
16+
) and
17+
sink.asExpr().getFile().getStem() = "GuiceRequestParameters"
18+
}
19+
}
20+
21+
from Conf c, DataFlow::Node src, DataFlow::Node sink
22+
where c.hasFlow(src, sink)
23+
select src, sink

0 commit comments

Comments
 (0)