Skip to content

Commit c626890

Browse files
atorralbaatorralba
authored
Merge pull request github#13256 from atorralba/atorralba/java/stapler-models
Java: Model the Stapler framework
2 parents 5cb451b + 1b39faa commit c626890

File tree

19 files changed

+340
-2
lines changed

19 files changed

+340
-2
lines changed

‎java/ql/lib/change-notes/2023-05-22-stapler-models.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added more models for the Stapler framework.

‎java/ql/lib/ext/org.kohsuke.stapler.bind.model.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/java-all
4+
extensible: sourceModel
5+
data:
6+
- ["org.kohsuke.stapler.bind", "JavaScriptMethod", True, "", "", "Annotated", "Parameter", "remote", "manual"]

‎java/ql/lib/ext/org.kohsuke.stapler.json.model.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/java-all
4+
extensible: sourceModel
5+
data:
6+
- ["org.kohsuke.stapler.json", "SubmittedForm", True, "", "", "Annotated", "Parameter", "remote", "manual"]
7+
- ["org.kohsuke.stapler.json", "JsonBody", True, "", "", "Annotated", "Parameter", "remote", "manual"]

‎java/ql/lib/ext/org.kohsuke.stapler.model.yml

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,46 @@ extensions:
44
extensible: sinkModel
55
data:
66
- ["org.kohsuke.stapler", "HttpResponses", True, "redirectTo", "(String)", "", "Argument[0]", "url-redirection", "ai-manual"]
7+
- ["org.kohsuke.stapler", "HttpResponses", True, "redirectTo", "(int,String)", "", "Argument[1]", "url-redirection", "manual"]
78
- ["org.kohsuke.stapler", "HttpResponses", True, "staticResource", "(URL)", "", "Argument[0]", "request-forgery", "ai-manual"]
9+
- ["org.kohsuke.stapler", "HttpResponses", True, "staticResource", "(URL,long)", "", "Argument[0]", "request-forgery", "manual"]
10+
- ["org.kohsuke.stapler", "HttpResponses", True, "html", "(String)", "", "Argument[0]", "html-injection", "manual"]
11+
- ["org.kohsuke.stapler", "HttpResponses", True, "literalHtml", "(String)", "", "Argument[0]", "html-injection", "manual"]
12+
- ["org.kohsuke.stapler", "StaplerResponse", True, "forward", "(Object,String,StaplerRequest)", "", "Argument[1]", "request-forgery", "manual"]
13+
- ["org.kohsuke.stapler", "StaplerResponse", True, "sendRedirect2", "(String)", "", "Argument[0]", "url-redirection", "manual"]
14+
- ["org.kohsuke.stapler", "StaplerResponse", True, "sendRedirect", "(int,String)", "", "Argument[1]", "url-redirection", "manual"]
15+
- ["org.kohsuke.stapler", "StaplerResponse", True, "sendRedirect", "(String)", "", "Argument[0]", "url-redirection", "manual"]
16+
- ["org.kohsuke.stapler", "StaplerResponse", True, "serveFile", "(StaplerRequest,URL)", "", "Argument[1]", "path-injection", "manual"]
17+
- ["org.kohsuke.stapler", "StaplerResponse", True, "serveFile", "(StaplerRequest,URL,long)", "", "Argument[1]", "path-injection", "manual"]
18+
- ["org.kohsuke.stapler", "StaplerResponse", True, "serveLocalizedFile", "(StaplerRequest,URL)", "", "Argument[1]", "path-injection", "manual"]
19+
- ["org.kohsuke.stapler", "StaplerResponse", True, "serveLocalizedFile", "(StaplerRequest,URL,long)", "", "Argument[1]", "path-injection", "manual"]
20+
- ["org.kohsuke.stapler", "StaplerResponse", True, "serveFile", "(StaplerRequest,InputStream,long,long,long,String)", "", "Argument[1]", "path-injection", "manual"]
21+
- ["org.kohsuke.stapler", "StaplerResponse", True, "serveFile", "(StaplerRequest,InputStream,long,long,int,String)", "", "Argument[1]", "path-injection", "manual"]
22+
- ["org.kohsuke.stapler", "StaplerResponse", True, "serveFile", "(StaplerRequest,InputStream,long,long,String)", "", "Argument[1]", "path-injection", "manual"]
23+
- ["org.kohsuke.stapler", "StaplerResponse", True, "serveFile", "(StaplerRequest,InputStream,long,int,String)", "", "Argument[1]", "path-injection", "manual"]
24+
- ["org.kohsuke.stapler", "StaplerResponse", True, "reverseProxyTo", "(URL,StaplerRequest)", "", "Argument[0]", "request-forgery", "manual"]
25+
- addsTo:
26+
pack: codeql/java-all
27+
extensible: sourceModel
28+
data:
29+
- ["org.kohsuke.stapler", "StaplerRequest", True, "getRequestURIWithQueryString", "", "", "ReturnValue", "remote", "manual"]
30+
- ["org.kohsuke.stapler", "StaplerRequest", True, "getRequestURLWithQueryString", "", "", "ReturnValue", "remote", "manual"]
31+
- ["org.kohsuke.stapler", "StaplerRequest", True, "getReferer", "", "", "ReturnValue", "remote", "manual"]
32+
- ["org.kohsuke.stapler", "StaplerRequest", True, "getOriginalRequestURI", "", "", "ReturnValue", "remote", "manual"]
33+
- ["org.kohsuke.stapler", "StaplerRequest", True, "getSubmittedForm", "", "", "ReturnValue", "remote", "manual"]
34+
- ["org.kohsuke.stapler", "StaplerRequest", True, "getFileItem", "", "", "ReturnValue", "remote", "manual"]
35+
- ["org.kohsuke.stapler", "StaplerRequest", True, "bindParametersToList", "", "", "ReturnValue", "remote", "manual"]
36+
- ["org.kohsuke.stapler", "StaplerRequest", True, "bindParameters", "", "", "Argument[0]", "remote", "manual"]
37+
- ["org.kohsuke.stapler", "StaplerRequest", True, "bindParameters", "", "", "ReturnValue", "remote", "manual"]
38+
- ["org.kohsuke.stapler", "StaplerRequest", True, "bindJSON", "", "", "Argument[0]", "remote", "manual"]
39+
- ["org.kohsuke.stapler", "StaplerRequest", True, "bindJSON", "", "", "ReturnValue", "remote", "manual"]
40+
- ["org.kohsuke.stapler", "StaplerRequest", True, "bindJSONToList", "", "", "ReturnValue", "remote", "manual"]
41+
- ["org.kohsuke.stapler", "StaplerRequest", True, "getParameter", "", "", "ReturnValue", "remote", "manual"]
42+
- ["org.kohsuke.stapler", "StaplerRequest", True, "getParameterMap", "", "", "ReturnValue", "remote", "manual"]
43+
- ["org.kohsuke.stapler", "StaplerRequest", True, "getParameterNames", "", "", "ReturnValue", "remote", "manual"]
44+
- ["org.kohsuke.stapler", "StaplerRequest", True, "getParameterValues", "", "", "ReturnValue", "remote", "manual"]
45+
- ["org.kohsuke.stapler", "StaplerRequest", True, "getRestOfPath", "", "", "ReturnValue", "remote", "manual"]
46+
- ["org.kohsuke.stapler", "QueryParameter", True, "", "", "Annotated", "Parameter", "remote", "manual"]
47+
- ["org.kohsuke.stapler", "Header", True, "", "", "Annotated", "Parameter", "remote", "manual"]
48+
- ["org.kohsuke.stapler", "DataBoundConstructor", True, "", "", "Annotated", "Parameter", "remote", "manual"]
49+
- ["org.kohsuke.stapler", "DataBoundSetter", True, "", "", "Annotated", "Parameter", "remote", "manual"]

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ abstract class RemoteFlowSource extends DataFlow::Node {
4141
*/
4242
private module FlowSources {
4343
private import semmle.code.java.frameworks.hudson.Hudson
44+
private import semmle.code.java.frameworks.stapler.Stapler
4445
}
4546

4647
private class ExternalRemoteFlowSource extends RemoteFlowSource {

‎java/ql/lib/semmle/code/java/dataflow/FlowSteps.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ private module Frameworks {
2323
private import semmle.code.java.frameworks.Properties
2424
private import semmle.code.java.frameworks.Protobuf
2525
private import semmle.code.java.frameworks.ratpack.RatpackExec
26+
private import semmle.code.java.frameworks.stapler.Stapler
2627
private import semmle.code.java.JDK
2728
}
2829

‎java/ql/lib/semmle/code/java/frameworks/hudson/Hudson.qll

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,17 @@
22

33
import java
44
private import semmle.code.java.dataflow.FlowSources
5+
private import semmle.code.java.frameworks.stapler.Stapler
56
private import semmle.code.java.security.XSS
67

8+
/** A method declared in a subtype of `hudson.model.Descriptor` that returns an `HttpResponse`. */
9+
class HudsonWebMethod extends Method {
10+
HudsonWebMethod() {
11+
this.getReturnType().(RefType).getASourceSupertype*() instanceof HttpResponse and
12+
this.getDeclaringType().getASourceSupertype*().hasQualifiedName("hudson.model", "Descriptor")
13+
}
14+
}
15+
716
private class FilePathRead extends LocalUserInput {
817
FilePathRead() {
918
this.asExpr()

‎java/ql/lib/semmle/code/java/frameworks/stapler/Stapler.qll

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
/** Provides classes and predicates related to the Stapler framework. */
2+
3+
import java
4+
private import semmle.code.java.dataflow.DataFlow
5+
private import semmle.code.java.dataflow.FlowSources
6+
private import semmle.code.java.dataflow.FlowSteps
7+
private import semmle.code.java.dataflow.TypeFlow
8+
private import semmle.code.java.frameworks.hudson.Hudson
9+
private import semmle.code.java.frameworks.JavaxAnnotations
10+
11+
/**
12+
* A callable annotated with a Stapler `DataBound` annotation,
13+
* or that has the `@stapler-constructor` Javadoc annotation.
14+
*/
15+
class DataBoundAnnotated extends Callable {
16+
DataBoundAnnotated() {
17+
exists(Annotation an |
18+
an.getType()
19+
.hasQualifiedName("org.kohsuke.stapler", ["DataBoundConstructor", "DataBoundSetter"])
20+
|
21+
this = an.getAnnotatedElement()
22+
)
23+
or
24+
exists(Javadoc doc | doc.getAChild().getText().matches("%@stapler-constructor%") |
25+
doc.getCommentedElement() = this
26+
)
27+
}
28+
}
29+
30+
/** The interface `org.kohsuke.stapler.HttpResponse`. */
31+
class HttpResponse extends Interface {
32+
HttpResponse() { this.hasQualifiedName("org.kohsuke.stapler", "HttpResponse") }
33+
}
34+
35+
/**
36+
* A remote flow source for parameters annotated with an annotation
37+
* that is itself annotated with `InjectedParameter`.
38+
*
39+
* Such parameters are populated with user-provided data by Stapler.
40+
*/
41+
private class InjectedParameterSource extends RemoteFlowSource {
42+
InjectedParameterSource() {
43+
this.asParameter().getAnAnnotation().getType() instanceof InjectedParameterAnnotatedType
44+
}
45+
46+
override string getSourceType() { result = "Stapler injected parameter" }
47+
}
48+
49+
/**
50+
* A dataflow step from the `HttpResponse` return value of a `HudsonWebMethod`
51+
* to the instance parameter of the `generateResponse` method of the appropriate subtype of `HttpResponse`.
52+
*
53+
* This models the rendering process of an `HttpResponse` by Stapler.
54+
*/
55+
private class HttpResponseGetDescriptionStep extends AdditionalValueStep {
56+
override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
57+
exists(ReturnStmt s, GenerateResponseMethod m |
58+
s.getEnclosingCallable() instanceof HudsonWebMethod and
59+
boundOrStaticType(s.getResult(), m.getDeclaringType().getADescendant())
60+
|
61+
n1.asExpr() = s.getResult() and
62+
n2.(DataFlow::InstanceParameterNode).getCallable() = m
63+
)
64+
}
65+
}
66+
67+
/**
68+
* A dataflow step from the post-update node of an instance access in a `DataBoundAnnotated` method
69+
* to the instance parameter of a `PostConstruct` method of the same type.
70+
*
71+
* This models the construction process of a `DataBound` object in Stapler.
72+
*/
73+
private class PostConstructDataBoundAdditionalStep extends AdditionalValueStep {
74+
override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
75+
exists(PostConstructDataBoundMethod postConstruct, DataBoundAnnotated input |
76+
postConstruct.getDeclaringType() = input.getDeclaringType()
77+
|
78+
n1.(DataFlow::PostUpdateNode)
79+
.getPreUpdateNode()
80+
.(DataFlow::InstanceAccessNode)
81+
.getEnclosingCallable() = input and
82+
n2.(DataFlow::InstanceParameterNode).getCallable() = postConstruct
83+
)
84+
}
85+
}
86+
87+
/** An annotation type annotated with the `InjectedParameter` annotation. */
88+
private class InjectedParameterAnnotatedType extends AnnotationType {
89+
InjectedParameterAnnotatedType() {
90+
this.getAnAnnotation().getType().hasQualifiedName("org.kohsuke.stapler", "InjectedParameter")
91+
}
92+
}
93+
94+
/** The `generateResponse` method of `org.kohsuke.stapler.HttpResponse` or its subtypes. */
95+
private class GenerateResponseMethod extends Method {
96+
GenerateResponseMethod() {
97+
this.getDeclaringType().getASourceSupertype*() instanceof HttpResponse and
98+
this.hasName("generateResponse")
99+
}
100+
}
101+
102+
/** Holds if `t` is the static type of `e`, or an upper bound of the runtime type of `e`. */
103+
private predicate boundOrStaticType(Expr e, RefType t) {
104+
exprTypeFlow(e, t, false)
105+
or
106+
t = e.getType()
107+
}
108+
109+
/**
110+
* A method called after the construction of a `DataBound` object.
111+
*
112+
* That is, either the `bindResolve` method of a subtype of `org.kohsuke.stapler.DataBoundResolvable`,
113+
* or a method annotated with `javax.annotation.PostConstruct`.
114+
*/
115+
private class PostConstructDataBoundMethod extends Method {
116+
PostConstructDataBoundMethod() {
117+
this.getDeclaringType()
118+
.getASourceSupertype*()
119+
.hasQualifiedName("org.kohsuke.stapler", "DataBoundResolvable") and
120+
this.hasName("bindResolve")
121+
or
122+
this.getAnAnnotation() instanceof PostConstructAnnotation
123+
}
124+
}

‎java/ql/test/library-tests/dataflow/taintsources/Stapler.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import org.kohsuke.stapler.InjectedParameter;
2+
3+
public class Stapler {
4+
5+
@InjectedParameter
6+
private @interface MyInjectedParameter {
7+
}
8+
9+
private static void sink(Object o) {}
10+
11+
public static void test(@MyInjectedParameter String src) {
12+
sink(src); // $ hasRemoteValueFlow
13+
}
14+
}

‎java/ql/test/library-tests/dataflow/taintsources/options

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/servlet-api-2.4:${testdir}/../../../stubs/springframework-5.3.8:${testdir}/../../../stubs/google-android-9.0.0:${testdir}/../../../stubs/playframework-2.6.x:${testdir}/../../../stubs/jackson-databind-2.12:${testdir}/../../../stubs/jackson-core-2.12:${testdir}/../../../stubs/akka-2.6.x:${testdir}/../../../stubs/jwtk-jjwt-0.11.2:${testdir}/../../../stubs/jenkins
1+
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/servlet-api-2.4:${testdir}/../../../stubs/springframework-5.3.8:${testdir}/../../../stubs/google-android-9.0.0:${testdir}/../../../stubs/playframework-2.6.x:${testdir}/../../../stubs/jackson-databind-2.12:${testdir}/../../../stubs/jackson-core-2.12:${testdir}/../../../stubs/akka-2.6.x:${testdir}/../../../stubs/jwtk-jjwt-0.11.2:${testdir}/../../../stubs/jenkins:${testdir}/../../../stubs/stapler-1.263

0 commit comments

Comments
 (0)