Skip to content

Commit d1f21a8

Browse files
authored
Merge pull request github#6042 from joefarebrother/spring-http
[Java] Model spring `http` package
2 parents 12f7921 + f7de2e6 commit d1f21a8

File tree

17 files changed

+719
-529
lines changed

17 files changed

+719
-529
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
lgtm,codescanning
2+
* Additional flow steps in the `org.springframework.http` package of the Spring framework have been modelled.
3+
This may result in additional results for security queries on projects using this framework.

java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ private module Frameworks {
105105
private import semmle.code.java.frameworks.MyBatis
106106
private import semmle.code.java.frameworks.Hibernate
107107
private import semmle.code.java.frameworks.jOOQ
108+
private import semmle.code.java.frameworks.spring.SpringHttp
108109
}
109110

110111
private predicate sourceModelCsv(string row) {

java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -209,22 +209,6 @@ private predicate constructorStep(Expr tracked, ConstructorCall sink) {
209209
// a custom InputStream that wraps a tainted data source is tainted
210210
inputStreamWrapper(sink.getConstructor(), argi)
211211
or
212-
// A SpringHttpEntity is a wrapper around a body and some headers
213-
// Track flow through iff body is a String
214-
exists(SpringHttpEntity she |
215-
sink.getConstructor() = she.getAConstructor() and
216-
argi = 0 and
217-
tracked.getType() instanceof TypeString
218-
)
219-
or
220-
// A SpringRequestEntity is a wrapper around a body and some headers
221-
// Track flow through iff body is a String
222-
exists(SpringResponseEntity sre |
223-
sink.getConstructor() = sre.getAConstructor() and
224-
argi = 0 and
225-
tracked.getType() instanceof TypeString
226-
)
227-
or
228212
sink.getConstructor().(TaintPreservingCallable).returnsTaintFrom(argToParam(sink, argi))
229213
)
230214
}
@@ -277,19 +261,6 @@ private predicate taintPreservingQualifierToMethod(Method m) {
277261
m.getDeclaringType().getASubtype*() instanceof SpringUntrustedDataType and
278262
not m.getDeclaringType() instanceof TypeObject
279263
or
280-
m.getDeclaringType() instanceof SpringHttpEntity and
281-
m.getName().regexpMatch("getBody|getHeaders")
282-
or
283-
exists(SpringHttpHeaders headers | m = headers.getAMethod() |
284-
m.getReturnType() instanceof TypeString
285-
or
286-
exists(ParameterizedType stringlist |
287-
m.getReturnType().(RefType).getASupertype*() = stringlist and
288-
stringlist.getSourceDeclaration().hasQualifiedName("java.util", "List") and
289-
stringlist.getTypeArgument(0) instanceof TypeString
290-
)
291-
)
292-
or
293264
m.(TaintPreservingCallable).returnsTaintFrom(-1)
294265
or
295266
exists(JaxRsResourceMethod resourceMethod |

java/ql/src/semmle/code/java/frameworks/spring/SpringHttp.qll

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,3 +61,82 @@ private class UrlOpenSink extends SinkModelCsv {
6161
]
6262
}
6363
}
64+
65+
private class SpringHttpFlowStep extends SummaryModelCsv {
66+
override predicate row(string row) {
67+
row =
68+
[
69+
//"package;type;overrides;name;signature;ext;inputspec;outputspec;kind",
70+
"org.springframework.http;HttpEntity;true;HttpEntity;(Object);;Argument[0];Argument[-1];taint",
71+
"org.springframework.http;HttpEntity;true;HttpEntity;(Object,MultiValueMap);;Argument[0];Argument[-1];taint",
72+
"org.springframework.http;HttpEntity;true;HttpEntity;(Object,MultiValueMap);;MapKey of Argument[1];Argument[-1];taint",
73+
"org.springframework.http;HttpEntity;true;HttpEntity;(Object,MultiValueMap);;Element of MapValue of Argument[1];Argument[-1];taint",
74+
"org.springframework.http;HttpEntity;true;HttpEntity;(MultiValueMap);;MapKey of Argument[0];Argument[-1];taint",
75+
"org.springframework.http;HttpEntity;true;HttpEntity;(MultiValueMap);;Element of MapValue of Argument[0];Argument[-1];taint",
76+
"org.springframework.http;HttpEntity;true;getBody;;;Argument[-1];ReturnValue;taint",
77+
"org.springframework.http;HttpEntity;true;getHeaders;;;Argument[-1];ReturnValue;taint",
78+
"org.springframework.http;ResponseEntity;true;ResponseEntity;(Object,HttpStatus);;Argument[0];Argument[-1];taint",
79+
"org.springframework.http;ResponseEntity;true;ResponseEntity;(Object,MultiValueMap,HttpStatus);;Argument[0];Argument[-1];taint",
80+
"org.springframework.http;ResponseEntity;true;ResponseEntity;(Object,MultiValueMap,HttpStatus);;MapKey of Argument[1];Argument[-1];taint",
81+
"org.springframework.http;ResponseEntity;true;ResponseEntity;(Object,MultiValueMap,HttpStatus);;Element of MapValue of Argument[1];Argument[-1];taint",
82+
"org.springframework.http;ResponseEntity;true;ResponseEntity;(MultiValueMap,HttpStatus);;MapKey of Argument[0];Argument[-1];taint",
83+
"org.springframework.http;ResponseEntity;true;ResponseEntity;(MultiValueMap,HttpStatus);;Element of MapValue of Argument[0];Argument[-1];taint",
84+
"org.springframework.http;ResponseEntity;true;ResponseEntity;(Object,MultiValueMap,int);;Argument[0];Argument[-1];taint",
85+
"org.springframework.http;ResponseEntity;true;ResponseEntity;(Object,MultiValueMap,int);;MapKey of Argument[1];Argument[-1];taint",
86+
"org.springframework.http;ResponseEntity;true;ResponseEntity;(Object,MultiValueMap,int);;Element of MapValue of Argument[1];Argument[-1];taint",
87+
"org.springframework.http;ResponseEntity;true;of;(Optional);;Element of Argument[0];ReturnValue;taint",
88+
"org.springframework.http;ResponseEntity;true;ok;(Object);;Argument[0];ReturnValue;taint",
89+
"org.springframework.http;ResponseEntity;true;created;(URI);;Argument[0];ReturnValue;taint",
90+
"org.springframework.http;ResponseEntity$BodyBuilder;true;contentLength;(long);;Argument[-1];ReturnValue;value",
91+
"org.springframework.http;ResponseEntity$BodyBuilder;true;contentType;(MediaType);;Argument[-1];ReturnValue;value",
92+
"org.springframework.http;ResponseEntity$BodyBuilder;true;body;(Object);;Argument[-1..0];ReturnValue;taint",
93+
"org.springframework.http;ResponseEntity$HeadersBuilder;true;allow;(HttpMethod[]);;Argument[-1];ReturnValue;value",
94+
"org.springframework.http;ResponseEntity$HeadersBuilder;true;eTag;(String);;Argument[-1];ReturnValue;value",
95+
"org.springframework.http;ResponseEntity$HeadersBuilder;true;eTag;(String);;Argument[0];Argument[-1];taint",
96+
"org.springframework.http;ResponseEntity$HeadersBuilder;true;header;(String,String[]);;Argument[-1];ReturnValue;value",
97+
"org.springframework.http;ResponseEntity$HeadersBuilder;true;header;(String,String[]);;Argument[0];Argument[-1];taint",
98+
"org.springframework.http;ResponseEntity$HeadersBuilder;true;header;(String,String[]);;ArrayElement of Argument[1];Argument[-1];taint",
99+
"org.springframework.http;ResponseEntity$HeadersBuilder;true;headers;(Consumer);;Argument[-1];ReturnValue;value",
100+
"org.springframework.http;ResponseEntity$HeadersBuilder;true;headers;(HttpHeaders);;Argument[-1];ReturnValue;value",
101+
"org.springframework.http;ResponseEntity$HeadersBuilder;true;headers;(HttpHeaders);;Argument[0];Argument[-1];taint",
102+
"org.springframework.http;ResponseEntity$HeadersBuilder;true;lastModified;;;Argument[-1];ReturnValue;value",
103+
"org.springframework.http;ResponseEntity$HeadersBuilder;true;location;(URI);;Argument[-1];ReturnValue;value",
104+
"org.springframework.http;ResponseEntity$HeadersBuilder;true;location;(URI);;Argument[0];Argument[-1];taint",
105+
"org.springframework.http;ResponseEntity$HeadersBuilder;true;varyBy;(String[]);;Argument[-1];ReturnValue;value",
106+
"org.springframework.http;ResponseEntity$HeadersBuilder;true;build;();;Argument[-1];ReturnValue;taint",
107+
"org.springframework.http;RequestEntity;true;getUrl;();;Argument[-1];ReturnValue;taint",
108+
"org.springframework.http;HttpHeaders;true;HttpHeaders;(MultiValueMap);;MapKey of Argument[0];Argument[-1];taint",
109+
"org.springframework.http;HttpHeaders;true;HttpHeaders;(MultiValueMap);;Element of MapValue of Argument[0];Argument[-1];taint",
110+
"org.springframework.http;HttpHeaders;true;get;(Object);;Argument[-1];Element of ReturnValue;taint",
111+
"org.springframework.http;HttpHeaders;true;getAccessControlAllowHeaders;();;Argument[-1];Element of ReturnValue;taint",
112+
"org.springframework.http;HttpHeaders;true;getAccessControlAllowOrigin;();;Argument[-1];ReturnValue;taint",
113+
"org.springframework.http;HttpHeaders;true;getAccessControlExposeHeaders;();;Argument[-1];Element of ReturnValue;taint",
114+
"org.springframework.http;HttpHeaders;true;getAccessControlRequestHeaders;();;Argument[-1];Element of ReturnValue;taint",
115+
"org.springframework.http;HttpHeaders;true;getCacheControl;();;Argument[-1];ReturnValue;taint",
116+
"org.springframework.http;HttpHeaders;true;getConnection;();;Argument[-1];Element of ReturnValue;taint",
117+
"org.springframework.http;HttpHeaders;true;getETag;();;Argument[-1];ReturnValue;taint",
118+
"org.springframework.http;HttpHeaders;true;getETagValuesAsList;(String);;Argument[-1];Element of ReturnValue;taint",
119+
"org.springframework.http;HttpHeaders;true;getFieldValues;(String);;Argument[-1];ReturnValue;taint",
120+
"org.springframework.http;HttpHeaders;true;getFirst;(String);;Argument[-1];ReturnValue;taint",
121+
"org.springframework.http;HttpHeaders;true;getIfMatch;();;Argument[-1];Element of ReturnValue;taint",
122+
"org.springframework.http;HttpHeaders;true;getIfNoneMatch;();;Argument[-1];Element of ReturnValue;taint",
123+
"org.springframework.http;HttpHeaders;true;getHost;();;Argument[-1];ReturnValue;taint",
124+
"org.springframework.http;HttpHeaders;true;getLocation;();;Argument[-1];ReturnValue;taint",
125+
"org.springframework.http;HttpHeaders;true;getOrEmpty;(Object);;Argument[-1];Element of ReturnValue;taint",
126+
"org.springframework.http;HttpHeaders;true;getOrigin;();;Argument[-1];ReturnValue;taint",
127+
"org.springframework.http;HttpHeaders;true;getPragma;();;Argument[-1];ReturnValue;taint",
128+
"org.springframework.http;HttpHeaders;true;getUpgrade;();;Argument[-1];ReturnValue;taint",
129+
"org.springframework.http;HttpHeaders;true;getValuesAsList;(String);;Argument[-1];Element of ReturnValue;taint",
130+
"org.springframework.http;HttpHeaders;true;getVary;();;Argument[-1];Element of ReturnValue;taint",
131+
"org.springframework.http;HttpHeaders;true;add;(String,String);;Argument[0..1];Argument[-1];taint",
132+
"org.springframework.http;HttpHeaders;true;set;(String,String);;Argument[0..1];Argument[-1];taint",
133+
"org.springframework.http;HttpHeaders;true;addAll;(MultiValueMap);;MapKey of Argument[0];Argument[-1];taint",
134+
"org.springframework.http;HttpHeaders;true;addAll;(MultiValueMap);;Element of MapValue of Argument[0];Argument[-1];taint",
135+
"org.springframework.http;HttpHeaders;true;addAll;(String,List);;Argument[0];Argument[-1];taint",
136+
"org.springframework.http;HttpHeaders;true;addAll;(String,List);;Element of Argument[1];Argument[-1];taint",
137+
"org.springframework.http;HttpHeaders;true;formatHeaders;(MultiValueMap);;MapKey of Argument[0];ReturnValue;taint",
138+
"org.springframework.http;HttpHeaders;true;formatHeaders;(MultiValueMap);;Element of MapValue of Argument[0];ReturnValue;taint",
139+
"org.springframework.http;HttpHeaders;true;encodeBasicAuth;(String,String,Charset);;Argument[0..1];ReturnValue;taint"
140+
]
141+
}
142+
}
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
import org.springframework.http.HttpEntity;
2+
import org.springframework.http.ResponseEntity;
3+
import org.springframework.http.RequestEntity;
4+
import org.springframework.http.HttpHeaders;
5+
import org.springframework.http.HttpStatus;
6+
import org.springframework.util.MultiValueMap;
7+
import org.springframework.util.LinkedMultiValueMap;
8+
import java.util.Optional;
9+
import java.util.List;
10+
11+
class TestHttp {
12+
static <T> T taint() { return null; }
13+
static void sink(Object o) {}
14+
15+
void test1() {
16+
String x = taint();
17+
sink(new HttpEntity(x)); // $hasTaintFlow
18+
19+
MultiValueMap<String,String> m1 = new LinkedMultiValueMap();
20+
sink(new HttpEntity(x, m1)); // $hasTaintFlow
21+
22+
m1.add("a", taint());
23+
sink(new HttpEntity("a", m1)); // $hasTaintFlow
24+
sink(new HttpEntity<String>(m1)); // $hasTaintFlow
25+
26+
MultiValueMap<String,String> m2 = new LinkedMultiValueMap();
27+
m2.add(taint(), "a");
28+
sink(new HttpEntity<String>(m2)); // $hasTaintFlow
29+
30+
HttpEntity<String> ent = taint();
31+
sink(ent.getBody()); // $hasTaintFlow
32+
sink(ent.getHeaders()); // $hasTaintFlow
33+
34+
RequestEntity<String> req = taint();
35+
sink(req.getUrl()); // $hasTaintFlow
36+
}
37+
38+
void test2() {
39+
String x = taint();
40+
sink(ResponseEntity.ok(x)); // $hasTaintFlow
41+
sink(ResponseEntity.of(Optional.of(x))); // $hasTaintFlow
42+
43+
sink(ResponseEntity.status(200).contentLength(2048).body(x)); // $hasTaintFlow
44+
sink(ResponseEntity.created(taint()).contentType(null).body("a")); // $hasTaintFlow
45+
sink(ResponseEntity.status(200).header(x, "a", "b", "c").build()); // $hasTaintFlow
46+
sink(ResponseEntity.status(200).header("h", "a", "b", x).build()); // $hasTaintFlow
47+
HttpHeaders h = new HttpHeaders();
48+
h.add("h", taint());
49+
sink(ResponseEntity.status(200).headers(h).allow().build()); // $hasTaintFlow
50+
sink(ResponseEntity.status(200).eTag(x).allow().build()); // $hasTaintFlow
51+
sink(ResponseEntity.status(200).location(taint()).lastModified(10000000).build()); // $hasTaintFlow
52+
sink(ResponseEntity.status(200).varyBy(x).build());
53+
}
54+
55+
void test3() {
56+
String x = taint();
57+
58+
MultiValueMap<String,String> m1 = new LinkedMultiValueMap();
59+
sink(new ResponseEntity(x, HttpStatus.ACCEPTED)); // $hasTaintFlow
60+
sink(new ResponseEntity(x, m1, HttpStatus.ACCEPTED)); // $hasTaintFlow
61+
sink(new ResponseEntity(x, m1, 200)); // $hasTaintFlow
62+
63+
m1.add("a", taint());
64+
sink(new ResponseEntity("a", m1, HttpStatus.ACCEPTED)); // $hasTaintFlow
65+
sink(new ResponseEntity<String>(m1, HttpStatus.ACCEPTED)); // $hasTaintFlow
66+
sink(new ResponseEntity("a", m1, 200)); // $hasTaintFlow
67+
68+
MultiValueMap<String,String> m2 = new LinkedMultiValueMap();
69+
m2.add(taint(), "a");
70+
sink(new ResponseEntity("a", m2, HttpStatus.ACCEPTED)); // $hasTaintFlow
71+
sink(new ResponseEntity<String>(m2, HttpStatus.ACCEPTED)); // $hasTaintFlow
72+
sink(new ResponseEntity("a", m2, 200)); // $hasTaintFlow
73+
74+
ResponseEntity<String> ent = taint();
75+
sink(ent.getBody()); // $hasTaintFlow
76+
sink(ent.getHeaders()); // $hasTaintFlow
77+
}
78+
79+
void test4() {
80+
MultiValueMap<String,String> m1 = new LinkedMultiValueMap();
81+
m1.add("a", taint());
82+
sink(new HttpHeaders(m1)); // $hasTaintFlow
83+
84+
MultiValueMap<String,String> m2 = new LinkedMultiValueMap();
85+
m2.add(taint(), "a");
86+
sink(new HttpHeaders(m2)); // $hasTaintFlow
87+
88+
HttpHeaders h1 = new HttpHeaders();
89+
h1.add(taint(), "a");
90+
sink(h1); // $hasTaintFlow
91+
92+
HttpHeaders h2 = new HttpHeaders();
93+
h2.add("a", taint());
94+
sink(h2); // $hasTaintFlow
95+
96+
HttpHeaders h3 = new HttpHeaders();
97+
h3.addAll(m1);
98+
sink(h3); // $hasTaintFlow
99+
100+
HttpHeaders h4 = new HttpHeaders();
101+
h4.addAll(m2);
102+
sink(h4); // $hasTaintFlow
103+
104+
HttpHeaders h5 = new HttpHeaders();
105+
h5.addAll(taint(), List.of());
106+
sink(h5); // $hasTaintFlow
107+
108+
HttpHeaders h6 = new HttpHeaders();
109+
h6.addAll("a", List.of(taint()));
110+
sink(h6); // $hasTaintFlow
111+
112+
sink(HttpHeaders.formatHeaders(m1)); // $hasTaintFlow
113+
sink(HttpHeaders.formatHeaders(m2)); // $hasTaintFlow
114+
115+
sink(HttpHeaders.encodeBasicAuth(taint(), "a", null)); // $hasTaintFlow
116+
sink(HttpHeaders.encodeBasicAuth("a", taint(), null)); // $hasTaintFlow
117+
}
118+
119+
void test5() {
120+
HttpHeaders h = taint();
121+
122+
sink(h.get(null).get(0)); // $hasTaintFlow
123+
sink(h.getAccept().get(0));
124+
sink(h.getAcceptCharset().get(0));
125+
sink(h.getAcceptLanguage().get(0));
126+
sink(h.getAcceptLanguageAsLocales().get(0));
127+
sink(h.getAccessControlAllowCredentials());
128+
sink(h.getAccessControlAllowHeaders().get(0)); // $hasTaintFlow
129+
sink(h.getAccessControlAllowMethods().get(0));
130+
sink(h.getAccessControlAllowOrigin()); // $hasTaintFlow
131+
sink(h.getAccessControlExposeHeaders().get(0)); // $hasTaintFlow
132+
sink(h.getAccessControlMaxAge());
133+
sink(h.getAccessControlRequestHeaders().get(0)); // $hasTaintFlow
134+
sink(h.getAccessControlRequestMethod());
135+
sink(h.getAllow().toArray()[0]);
136+
sink(h.getCacheControl()); // $hasTaintFlow
137+
sink(h.getConnection().get(0)); // $hasTaintFlow
138+
sink(h.getContentDisposition());
139+
sink(h.getContentLanguage());
140+
sink(h.getContentLength());
141+
sink(h.getContentType());
142+
sink(h.getDate());
143+
sink(h.getETag()); // $hasTaintFlow
144+
sink(h.getExpires());
145+
sink(h.getFirst("a")); // $hasTaintFlow
146+
sink(h.getFirstDate("a"));
147+
sink(h.getFirstZonedDateTime("a"));
148+
sink(h.getHost()); // $hasTaintFlow
149+
sink(h.getIfMatch().get(0)); // $hasTaintFlow
150+
sink(h.getIfModifiedSince());
151+
sink(h.getIfNoneMatch().get(0)); // $hasTaintFlow
152+
sink(h.getIfUnmodifiedSince());
153+
sink(h.getLastModified());
154+
sink(h.getLocation()); // $hasTaintFlow
155+
sink(h.getOrEmpty("a").get(0)); // $hasTaintFlow
156+
sink(h.getOrigin()); // $hasTaintFlow
157+
sink(h.getPragma()); // $hasTaintFlow
158+
sink(h.getUpgrade()); // $hasTaintFlow
159+
sink(h.getValuesAsList("a").get(0)); // $hasTaintFlow
160+
sink(h.getVary().get(0)); // $hasTaintFlow
161+
}
162+
}

java/ql/test/library-tests/frameworks/spring/http/flow.expected

Whitespace-only changes.
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import java
2+
import semmle.code.java.frameworks.spring.Spring
3+
import semmle.code.java.dataflow.TaintTracking
4+
import TestUtilities.InlineExpectationsTest
5+
6+
class TaintFlowConf extends TaintTracking::Configuration {
7+
TaintFlowConf() { this = "qltest:frameworks:spring-taint-flow" }
8+
9+
override predicate isSource(DataFlow::Node n) {
10+
exists(string name | name.matches("taint%") |
11+
n.asExpr().(MethodAccess).getMethod().hasName(name)
12+
)
13+
}
14+
15+
override predicate isSink(DataFlow::Node n) {
16+
exists(MethodAccess ma | ma.getMethod().hasName("sink") | n.asExpr() = ma.getAnArgument())
17+
}
18+
}
19+
20+
class ValueFlowConf extends DataFlow::Configuration {
21+
ValueFlowConf() { this = "qltest:frameworks:spring-value-flow" }
22+
23+
override predicate isSource(DataFlow::Node n) {
24+
n.asExpr().(MethodAccess).getMethod().hasName("taint")
25+
}
26+
27+
override predicate isSink(DataFlow::Node n) {
28+
exists(MethodAccess ma | ma.getMethod().hasName("sink") | n.asExpr() = ma.getAnArgument())
29+
}
30+
}
31+
32+
class HasFlowTest extends InlineExpectationsTest {
33+
HasFlowTest() { this = "HasFlowTest" }
34+
35+
override string getARelevantTag() { result = ["hasTaintFlow", "hasValueFlow"] }
36+
37+
override predicate hasActualResult(Location location, string element, string tag, string value) {
38+
tag = "hasTaintFlow" and
39+
exists(DataFlow::Node src, DataFlow::Node sink, TaintFlowConf conf | conf.hasFlow(src, sink) |
40+
not any(ValueFlowConf vconf).hasFlow(src, sink) and
41+
sink.getLocation() = location and
42+
element = sink.toString() and
43+
value = ""
44+
)
45+
or
46+
tag = "hasValueFlow" and
47+
exists(DataFlow::Node src, DataFlow::Node sink, ValueFlowConf conf | conf.hasFlow(src, sink) |
48+
sink.getLocation() = location and
49+
element = sink.toString() and
50+
value = ""
51+
)
52+
}
53+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.3.8

0 commit comments

Comments
 (0)