+
+
+
+ The Spring Expression Language (SpEL) is a powerful expression language
+ provided by Spring Framework. The language offers many features
+ including invocation of methods available in the JVM.
+
+
+ An unrestricted view name manipulation vulnerability in Spring Framework could lead to attacker-controlled arbitary SpEL expressions being evaluated using attacker-controlled data, which may in turn allow an attacker to run arbitrary code.
+
+
+ Note: two related variants of this problem are detected by different queries, `java/spring-view-manipulation` and `java/spring-view-manipulation-implicit`. The first detects taint flow problems where the return types is always String. While the latter, `java/spring-view-manipulation-implicit` detects cases where the request mapping method has a non-string return type such as void.
+
+
+
+
+
+ In general, using user input to determine Spring view name should be avoided.
+ If user input must be included in the expression, the controller can be annotated by
+ a @ReponseBody annotation. In this case, Spring Framework does not interpret
+ it as a view name, but just returns this string in HTTP Response. The same applies to using
+ a @RestController annotation on a class, as internally it inherits @ResponseBody.
+
+
+
+
+
+ In the following example, the Fragment method uses an externally controlled variable section to generate the view name. Hence, it is vulnerable to Spring View Manipulation attacks.
+
+
+
+ This can be easily prevented by using the ResponseBody annotation which marks the reponse is already processed preventing exploitation of Spring View Manipulation vulnerabilities. Alternatively, this can also be fixed by adding a HttpServletResponse parameter to the method definition as shown in the example below.
+
+
+
+
+
+
+ Veracode Research : Spring View Manipulation
+
+
+ Spring Framework Reference Documentation: Spring Expression Language (SpEL)
+
+
+ OWASP: Expression Language Injection
+
+
+
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewManipulation.ql b/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewManipulation.ql
new file mode 100644
index 00000000000..3c490e6bf68
--- /dev/null
+++ b/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewManipulation.ql
@@ -0,0 +1,21 @@
+/**
+ * @name Spring View Manipulation
+ * @description Untrusted input in a Spring View can lead to RCE.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/spring-view-manipulation
+ * @tags security
+ * external/cwe/cwe-094
+ */
+
+import java
+import SpringViewManipulationLib
+import DataFlow::PathGraph
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, SpringViewManipulationConfig conf
+where
+ thymeleafIsUsed() and
+ conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Potential Spring Expression Language injection from $@.",
+ source.getNode(), "this user input"
diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewManipulationLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewManipulationLib.qll
new file mode 100644
index 00000000000..b61d64ba7e1
--- /dev/null
+++ b/java/ql/src/experimental/Security/CWE/CWE-094/SpringViewManipulationLib.qll
@@ -0,0 +1,140 @@
+/**
+ * Provides classes for reasoning about Spring View Manipulation vulnerabilities
+ */
+
+import java
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.frameworks.spring.Spring
+import SpringFrameworkLib
+
+/** Holds if `Thymeleaf` templating engine is used in the project. */
+predicate thymeleafIsUsed() {
+ exists(Pom p |
+ p.getADependency().getArtifact().getValue() in [
+ "spring-boot-starter-thymeleaf", "thymeleaf-spring4", "springmvc-xml-thymeleaf",
+ "thymeleaf-spring5"
+ ]
+ )
+ or
+ exists(SpringBean b | b.getClassNameRaw().matches("org.thymeleaf.spring%"))
+}
+
+/** Models methods from the `javax.portlet.RenderState` package which return data from externally controlled sources. */
+class PortletRenderRequestMethod extends Method {
+ PortletRenderRequestMethod() {
+ exists(RefType c, Interface t |
+ c.extendsOrImplements*(t) and
+ t.hasQualifiedName("javax.portlet", "RenderState") and
+ this = c.getAMethod()
+ |
+ this.hasName([
+ "getCookies", "getParameter", "getRenderParameters", "getParameterNames",
+ "getParameterValues", "getParameterMap"
+ ])
+ )
+ }
+}
+
+/**
+ * A taint-tracking configuration for unsafe user input
+ * that can lead to Spring View Manipulation vulnerabilities.
+ */
+class SpringViewManipulationConfig extends TaintTracking::Configuration {
+ SpringViewManipulationConfig() { this = "Spring View Manipulation Config" }
+
+ override predicate isSource(DataFlow::Node source) {
+ source instanceof RemoteFlowSource or
+ source instanceof WebRequestSource or
+ source.asExpr().(MethodAccess).getMethod() instanceof PortletRenderRequestMethod
+ }
+
+ override predicate isSink(DataFlow::Node sink) { sink instanceof SpringViewManipulationSink }
+
+ override predicate isSanitizer(DataFlow::Node node) {
+ // Block flows like
+ // ```
+ // a = "redirect:" + taint`
+ // ```
+ exists(AddExpr e, StringLiteral sl |
+ node.asExpr() = e.getControlFlowNode().getASuccessor*() and
+ sl = e.getLeftOperand*() and
+ sl.getRepresentedString().matches(["redirect:%", "ajaxredirect:%", "forward:%"])
+ )
+ or
+ // Block flows like
+ // ```
+ // x.append("redirect:");
+ // x.append(tainted());
+ // return x.toString();
+ //
+ // "redirect:".concat(taint)
+ //
+ // String.format("redirect:%s",taint);
+ // ```
+ exists(Call ca, StringLiteral sl |
+ (
+ sl = ca.getArgument(_)
+ or
+ sl = ca.getQualifier()
+ ) and
+ ca = getAStringCombiningCall() and
+ sl.getRepresentedString().matches(["redirect:%", "ajaxredirect:%", "forward:%"])
+ |
+ exists(Call cc | DataFlow::localExprFlow(ca.getQualifier(), cc.getQualifier()) |
+ cc = node.asExpr()
+ )
+ )
+ }
+}
+
+private Call getAStringCombiningCall() {
+ exists(StringCombiningMethod m | result = m.getAReference())
+}
+
+abstract private class StringCombiningMethod extends Method { }
+
+private class AppendableAppendMethod extends StringCombiningMethod {
+ AppendableAppendMethod() {
+ exists(RefType t |
+ t.hasQualifiedName("java.lang", "Appendable") and
+ this.getDeclaringType().extendsOrImplements*(t) and
+ this.hasName("append")
+ )
+ }
+}
+
+private class StringConcatMethod extends StringCombiningMethod {
+ StringConcatMethod() {
+ this.getDeclaringType() instanceof TypeString and
+ this.hasName("concat")
+ }
+}
+
+private class StringFormatMethod extends StringCombiningMethod {
+ StringFormatMethod() {
+ this.getDeclaringType() instanceof TypeString and
+ this.hasName("format")
+ }
+}
+
+/**
+ * A sink for Spring View Manipulation vulnerabilities,
+ */
+class SpringViewManipulationSink extends DataFlow::ExprNode {
+ SpringViewManipulationSink() {
+ exists(ReturnStmt r, SpringRequestMappingMethod m |
+ r.getResult() = this.asExpr() and
+ m.getBody().getAStmt() = r and
+ not m.isResponseBody() and
+ r.getResult().getType() instanceof TypeString
+ )
+ or
+ exists(ConstructorCall c | c.getConstructedType() instanceof ModelAndView |
+ this.asExpr() = c.getArgument(0) and
+ c.getConstructor().getParameterType(0) instanceof TypeString
+ )
+ or
+ exists(SpringModelAndViewSetViewNameCall c | this.asExpr() = c.getArgument(0))
+ }
+}
diff --git a/java/ql/src/semmle/code/java/frameworks/spring/SpringController.qll b/java/ql/src/semmle/code/java/frameworks/spring/SpringController.qll
index 3abf325dfa2..f935a4803a9 100644
--- a/java/ql/src/semmle/code/java/frameworks/spring/SpringController.qll
+++ b/java/ql/src/semmle/code/java/frameworks/spring/SpringController.qll
@@ -126,6 +126,11 @@ class SpringRequestMappingMethod extends SpringControllerMethod {
requestMappingAnnotation.getValue("produces").(CompileTimeConstantExpr).getStringValue()
}
+ /** Gets the "value" @RequestMapping annotation value, if present. */
+ string getValue() {
+ result = requestMappingAnnotation.getValue("value").(CompileTimeConstantExpr).getStringValue()
+ }
+
/** Holds if this is considered an `@ResponseBody` method. */
predicate isResponseBody() {
getAnAnnotation().getType() instanceof SpringResponseBodyAnnotationType or
diff --git a/java/ql/src/semmle/code/java/frameworks/spring/SpringWeb.qll b/java/ql/src/semmle/code/java/frameworks/spring/SpringWeb.qll
index 4a71c71295e..dd8e660fd26 100644
--- a/java/ql/src/semmle/code/java/frameworks/spring/SpringWeb.qll
+++ b/java/ql/src/semmle/code/java/frameworks/spring/SpringWeb.qll
@@ -17,3 +17,23 @@ class SpringNativeWebRequest extends Class {
this.hasQualifiedName("org.springframework.web.context.request", "NativeWebRequest")
}
}
+
+/**
+ * A Spring `ModelAndView` class. This is either
+ * `org.springframework.web.servlet.ModelAndView` or
+ * `org.springframework.web.portlet.ModelAndView`.
+ */
+class ModelAndView extends Class {
+ ModelAndView() {
+ hasQualifiedName(["org.springframework.web.servlet", "org.springframework.web.portlet"],
+ "ModelAndView")
+ }
+}
+
+/** A call to the Spring `ModelAndView.setViewName` method. */
+class SpringModelAndViewSetViewNameCall extends MethodAccess {
+ SpringModelAndViewSetViewNameCall() {
+ getMethod().getDeclaringType() instanceof ModelAndView and
+ getMethod().hasName("setViewName")
+ }
+}