Skip to content

Commit 04244b3

Browse files
authored
Merge pull request github#5974 from github/sauyon/java/spring-webmultipart
Model Spring `web.multipart`
2 parents 3c4cd15 + 51211c0 commit 04244b3

28 files changed

+774
-0
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
lgtm,codescanning
2+
* Additional flow steps in the `org.springframework.web.multipart` package of the Spring framework
3+
have been modelled. This may result in additional results for security queries on projects using
4+
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
@@ -90,6 +90,7 @@ private module Frameworks {
9090
private import semmle.code.java.frameworks.spring.SpringValidation
9191
private import semmle.code.java.frameworks.spring.SpringWebClient
9292
private import semmle.code.java.frameworks.spring.SpringBeans
93+
private import semmle.code.java.frameworks.spring.SpringWebMultipart
9394
private import semmle.code.java.security.ResponseSplitting
9495
private import semmle.code.java.security.InformationLeak
9596
private import semmle.code.java.security.JexlInjectionSinkModels

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import semmle.code.java.frameworks.spring.SpringSet
3636
import semmle.code.java.frameworks.spring.SpringUtil
3737
import semmle.code.java.frameworks.spring.SpringValidation
3838
import semmle.code.java.frameworks.spring.SpringValue
39+
import semmle.code.java.frameworks.spring.SpringWebMultipart
3940
import semmle.code.java.frameworks.spring.SpringXMLElement
4041
import semmle.code.java.frameworks.spring.metrics.MetricSpringBean
4142
import semmle.code.java.frameworks.spring.metrics.MetricSpringBeanFile
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/** Provides models of taint flow in `org.springframework.web.multipart` */
2+
3+
import java
4+
private import semmle.code.java.dataflow.ExternalFlow
5+
6+
private class FlowSummaries extends SummaryModelCsv {
7+
override predicate row(string row) {
8+
row =
9+
[
10+
"org.springframework.web.multipart;MultipartFile;true;getBytes;;;Argument[-1];ReturnValue;taint",
11+
"org.springframework.web.multipart;MultipartFile;true;getInputStream;;;Argument[-1];ReturnValue;taint",
12+
"org.springframework.web.multipart;MultipartFile;true;getName;;;Argument[-1];ReturnValue;taint",
13+
"org.springframework.web.multipart;MultipartFile;true;getOriginalFilename;;;Argument[-1];ReturnValue;taint",
14+
"org.springframework.web.multipart;MultipartFile;true;getResource;;;Argument[-1];ReturnValue;taint",
15+
"org.springframework.web.multipart;MultipartHttpServletRequest;true;getMultipartHeaders;;;Argument[-1];ReturnValue;taint",
16+
"org.springframework.web.multipart;MultipartHttpServletRequest;true;getRequestHeaders;;;Argument[-1];ReturnValue;taint",
17+
"org.springframework.web.multipart;MultipartRequest;true;getFile;;;Argument[-1];ReturnValue;taint",
18+
"org.springframework.web.multipart;MultipartRequest;true;getFileMap;;;Argument[-1];MapValue of ReturnValue;taint",
19+
"org.springframework.web.multipart;MultipartRequest;true;getFileNames;;;Argument[-1];Element of ReturnValue;taint",
20+
"org.springframework.web.multipart;MultipartRequest;true;getFiles;;;Argument[-1];Element of ReturnValue;taint",
21+
"org.springframework.web.multipart;MultipartRequest;true;getMultiFileMap;;;Argument[-1];MapValue of ReturnValue;taint",
22+
"org.springframework.web.multipart;MultipartResolver;true;resolveMultipart;;;Argument[0];ReturnValue;taint"
23+
]
24+
}
25+
}
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
package generatedtest;
2+
3+
import java.io.InputStream;
4+
import java.util.Collection;
5+
import java.util.Iterator;
6+
import java.util.List;
7+
import java.util.Map;
8+
import javax.servlet.http.HttpServletRequest;
9+
import org.springframework.core.io.Resource;
10+
import org.springframework.http.HttpHeaders;
11+
import org.springframework.util.MultiValueMap;
12+
import org.springframework.web.multipart.MultipartFile;
13+
import org.springframework.web.multipart.MultipartHttpServletRequest;
14+
import org.springframework.web.multipart.MultipartRequest;
15+
import org.springframework.web.multipart.MultipartResolver;
16+
17+
// Test case generated by GenerateFlowTestCase.ql
18+
public class Test {
19+
20+
Object getElement(Iterator container) { return container.next(); }
21+
Object getElement(Collection container) { return container.iterator().next(); }
22+
Object getMapValue(Map container) { return container.get(null); }
23+
Object source() { return null; }
24+
void sink(Object o) { }
25+
26+
public void test() throws Exception {
27+
28+
{
29+
// "org.springframework.web.multipart;MultipartFile;true;getBytes;;;Argument[-1];ReturnValue;taint"
30+
byte[] out = null;
31+
MultipartFile in = (MultipartFile)source();
32+
out = in.getBytes();
33+
sink(out); // $hasTaintFlow
34+
}
35+
{
36+
// "org.springframework.web.multipart;MultipartFile;true;getInputStream;;;Argument[-1];ReturnValue;taint"
37+
InputStream out = null;
38+
MultipartFile in = (MultipartFile)source();
39+
out = in.getInputStream();
40+
sink(out); // $hasTaintFlow
41+
}
42+
{
43+
// "org.springframework.web.multipart;MultipartFile;true;getName;;;Argument[-1];ReturnValue;taint"
44+
String out = null;
45+
MultipartFile in = (MultipartFile)source();
46+
out = in.getName();
47+
sink(out); // $hasTaintFlow
48+
}
49+
{
50+
// "org.springframework.web.multipart;MultipartFile;true;getOriginalFilename;;;Argument[-1];ReturnValue;taint"
51+
String out = null;
52+
MultipartFile in = (MultipartFile)source();
53+
out = in.getOriginalFilename();
54+
sink(out); // $hasTaintFlow
55+
}
56+
{
57+
// "org.springframework.web.multipart;MultipartFile;true;getResource;;;Argument[-1];ReturnValue;taint"
58+
Resource out = null;
59+
MultipartFile in = (MultipartFile)source();
60+
out = in.getResource();
61+
sink(out); // $hasTaintFlow
62+
}
63+
{
64+
// "org.springframework.web.multipart;MultipartHttpServletRequest;true;getMultipartHeaders;;;Argument[-1];ReturnValue;taint"
65+
HttpHeaders out = null;
66+
MultipartHttpServletRequest in = (MultipartHttpServletRequest)source();
67+
out = in.getMultipartHeaders(null);
68+
sink(out); // $hasTaintFlow
69+
}
70+
{
71+
// "org.springframework.web.multipart;MultipartHttpServletRequest;true;getRequestHeaders;;;Argument[-1];ReturnValue;taint"
72+
HttpHeaders out = null;
73+
MultipartHttpServletRequest in = (MultipartHttpServletRequest)source();
74+
out = in.getRequestHeaders();
75+
sink(out); // $hasTaintFlow
76+
}
77+
{
78+
// "org.springframework.web.multipart;MultipartRequest;true;getFile;;;Argument[-1];ReturnValue;taint"
79+
MultipartFile out = null;
80+
MultipartRequest in = (MultipartRequest)source();
81+
out = in.getFile(null);
82+
sink(out); // $hasTaintFlow
83+
}
84+
{
85+
// "org.springframework.web.multipart;MultipartRequest;true;getFileMap;;;Argument[-1];MapValue of ReturnValue;taint"
86+
Map out = null;
87+
MultipartRequest in = (MultipartRequest)source();
88+
out = in.getFileMap();
89+
sink(getMapValue(out)); // $hasTaintFlow
90+
}
91+
{
92+
// "org.springframework.web.multipart;MultipartRequest;true;getFileNames;;;Argument[-1];Element of ReturnValue;taint"
93+
Iterator out = null;
94+
MultipartRequest in = (MultipartRequest)source();
95+
out = in.getFileNames();
96+
sink(getElement(out)); // $hasTaintFlow
97+
}
98+
{
99+
// "org.springframework.web.multipart;MultipartRequest;true;getFiles;;;Argument[-1];Element of ReturnValue;taint"
100+
List out = null;
101+
MultipartRequest in = (MultipartRequest)source();
102+
out = in.getFiles(null);
103+
sink(getElement(out)); // $hasTaintFlow
104+
}
105+
{
106+
// "org.springframework.web.multipart;MultipartRequest;true;getMultiFileMap;;;Argument[-1];MapValue of ReturnValue;taint"
107+
MultiValueMap out = null;
108+
MultipartRequest in = (MultipartRequest)source();
109+
out = in.getMultiFileMap();
110+
sink(getMapValue(out)); // $hasTaintFlow
111+
}
112+
{
113+
// "org.springframework.web.multipart;MultipartResolver;true;resolveMultipart;;;Argument[0];ReturnValue;taint"
114+
MultipartHttpServletRequest out = null;
115+
HttpServletRequest in = (HttpServletRequest)source();
116+
MultipartResolver instance = null;
117+
out = instance.resolveMultipart(in);
118+
sink(out); // $hasTaintFlow
119+
}
120+
121+
}
122+
123+
}
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:${testdir}/../../../../stubs/javax-servlet-2.5

java/ql/test/library-tests/frameworks/spring/webmultipart/test.expected

Whitespace-only changes.
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import java
2+
import semmle.code.java.dataflow.DataFlow
3+
import semmle.code.java.dataflow.TaintTracking
4+
import TestUtilities.InlineExpectationsTest
5+
6+
class ValueFlowConf extends DataFlow::Configuration {
7+
ValueFlowConf() { this = "qltest:valueFlowConf" }
8+
9+
override predicate isSource(DataFlow::Node n) {
10+
n.asExpr().(MethodAccess).getMethod().hasName("source")
11+
}
12+
13+
override predicate isSink(DataFlow::Node n) {
14+
n.asExpr().(Argument).getCall().getCallee().hasName("sink")
15+
}
16+
}
17+
18+
class TaintFlowConf extends TaintTracking::Configuration {
19+
TaintFlowConf() { this = "qltest:taintFlowConf" }
20+
21+
override predicate isSource(DataFlow::Node n) {
22+
n.asExpr().(MethodAccess).getMethod().hasName("source")
23+
}
24+
25+
override predicate isSink(DataFlow::Node n) {
26+
n.asExpr().(Argument).getCall().getCallee().hasName("sink")
27+
}
28+
}
29+
30+
class HasFlowTest extends InlineExpectationsTest {
31+
HasFlowTest() { this = "HasFlowTest" }
32+
33+
override string getARelevantTag() { result = ["hasValueFlow", "hasTaintFlow"] }
34+
35+
override predicate hasActualResult(Location location, string element, string tag, string value) {
36+
tag = "hasValueFlow" and
37+
exists(DataFlow::Node src, DataFlow::Node sink, ValueFlowConf conf | conf.hasFlow(src, sink) |
38+
sink.getLocation() = location and
39+
element = sink.toString() and
40+
value = ""
41+
)
42+
or
43+
tag = "hasTaintFlow" and
44+
exists(DataFlow::Node src, DataFlow::Node sink, TaintFlowConf conf |
45+
conf.hasFlow(src, sink) and not any(ValueFlowConf c).hasFlow(src, sink)
46+
|
47+
sink.getLocation() = location and
48+
element = sink.toString() and
49+
value = ""
50+
)
51+
}
52+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Generated automatically from javax.servlet.RequestDispatcher for testing purposes
2+
3+
package javax.servlet;
4+
5+
import javax.servlet.ServletRequest;
6+
import javax.servlet.ServletResponse;
7+
8+
public interface RequestDispatcher
9+
{
10+
void forward(ServletRequest p0, ServletResponse p1);
11+
void include(ServletRequest p0, ServletResponse p1);
12+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Generated automatically from javax.servlet.Servlet for testing purposes
2+
3+
package javax.servlet;
4+
5+
import javax.servlet.ServletConfig;
6+
import javax.servlet.ServletRequest;
7+
import javax.servlet.ServletResponse;
8+
9+
public interface Servlet
10+
{
11+
ServletConfig getServletConfig();
12+
String getServletInfo();
13+
void destroy();
14+
void init(ServletConfig p0);
15+
void service(ServletRequest p0, ServletResponse p1);
16+
}

0 commit comments

Comments
 (0)