github · smowton · Feb 23, 2022 · May 20, 2021 · Feb 22, 2022 · Feb 23, 2022
@@ -0,0 +1,19 @@
+@Controller
+public class VelocitySSTI {
+
+	@GetMapping(value = "bad")
+	public void bad(HttpServletRequest request) {
+		Velocity.init();
+
+		String code = request.getParameter("code");
+
+		VelocityContext context = new VelocityContext();
+
+		context.put("name", "Velocity");
+		context.put("project", "Jakarta");
+
+		StringWriter w = new StringWriter();
+		// evaluate( Context context, Writer out, String logTag, String instring )
+		Velocity.evaluate(context, w, "mystring", code);
+	}
+}
@@ -0,0 +1,17 @@
+@Controller
+public class VelocitySSTI {
+
+	@GetMapping(value = "good")
+	public void good(HttpServletRequest request) {
+		Velocity.init();
+		VelocityContext context = new VelocityContext();
+
+		context.put("name", "Velocity");
+		context.put("project", "Jakarta");
+
+		String s = "We are using $project $name to render this.";
+		StringWriter w = new StringWriter();
+		Velocity.evaluate(context, w, "mystring", s);
+		System.out.println(" string : " + w);
+	}
+}
@@ -0,0 +1,31 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p>
+            Template Injection occurs when user input is embedded in a template in an unsafe manner.
+            An attacker can use native template syntax to inject a malicious payload into a template, which is then executed server-side. This permits the attacker to run arbitrary code in the server's context.</p>
+  </overview>
+  <recommendation>
+    <p>
+            To fix this, ensure that an untrusted value is not used as a template. If the application requirements do not allow this, use a sandboxed environment where access to unsafe attributes and methods is prohibited.
+        </p>
+  </recommendation>
+  <example>
+    <p>
+      In the example given below, an untrusted HTTP parameter
+      <code>code</code>
+      is used as a Velocity template string. This can lead to remote code execution.
+    </p>
+    <sample src="SSTIBad.java" />
+
+    <p>
+      In the next example the problem is avoided by using a fixed template string
+      <code>s</code>
+      . Since, the template is not attacker controlled in this case, we prevent untrusted code execution.
+    </p>
+    <sample src="SSTIGood.java" />
+  </example>
+  <references>
+    <li>Portswigger : [Server Side Template Injection](https://portswigger.net/web-security/server-side-template-injection)</li>
+  </references>
+</qhelp>
@@ -0,0 +1,19 @@
+/**
+ * @name Server Side Template Injection
+ * @description Untrusted input used as a template parameter can lead to remote code execution.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/server-side-template-injection
+ * @tags security
+ *       external/cwe/cwe-094
+ */
+
+import java
+import TemplateInjection
+import DataFlow::PathGraph
+
+from TemplateInjectionFlowConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Potential arbitrary code execution due to $@.",
+  source.getNode(), "a template value loaded from a remote source."
@@ -0,0 +1,209 @@
+/** Definitions related to the Server Side Template Injection (SSTI) query. */
+
+import java
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.dataflow.FlowSources
+import experimental.semmle.code.java.frameworks.FreeMarker
+import experimental.semmle.code.java.frameworks.Velocity
+import experimental.semmle.code.java.frameworks.JinJava
+import experimental.semmle.code.java.frameworks.Pebble
+import experimental.semmle.code.java.frameworks.Thymeleaf
+
+/** A taint tracking configuration to reason about Server Side Template Injection (SSTI) vulnerabilities */
+class TemplateInjectionFlowConfig extends TaintTracking::Configuration {
+  TemplateInjectionFlowConfig() { this = "TemplateInjectionFlowConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node prev, DataFlow::Node succ) {
+    exists(AdditionalFlowStep a | a.isAdditionalTaintStep(prev, succ))
+  }
+}
+
+/**
+ * A data flow sink for Server Side Template Injection (SSTI) vulnerabilities
+ */
+abstract private class Sink extends DataFlow::ExprNode { }
+
+/**
+ * A data flow step for Server Side Template Injection (SSTI) vulnerabilities
+ */
+private class AdditionalFlowStep extends Unit {
+  abstract predicate isAdditionalTaintStep(DataFlow::Node prev, DataFlow::Node succ);
+}
+
+/**
+ * An argument to FreeMarker template engine's `process` method call.
+ */
+private class FreeMarkerProcessSink extends Sink {
+  FreeMarkerProcessSink() {
+    exists(MethodAccess m |
+      m.getCallee() instanceof MethodFreeMarkerTemplateProcess and
+      m.getArgument(0) = this.getExpr()
+    )
+  }
+}
+
+/**
+ * An reader passed an argument to FreeMarker template engine's `Template`
+ *   construtor call.
+ */
+private class FreeMarkerConstructorSink extends Sink {
+  FreeMarkerConstructorSink() {
+    // Template(java.lang.String name, java.io.Reader reader)
+    // Template(java.lang.String name, java.io.Reader reader, Configuration cfg)
+    // Template(java.lang.String name, java.io.Reader reader, Configuration cfg, java.lang.String encoding)
+    // Template(java.lang.String name, java.lang.String sourceName, java.io.Reader reader, Configuration cfg)
+    // Template(java.lang.String name, java.lang.String sourceName, java.io.Reader reader, Configuration cfg, ParserConfiguration customParserConfiguration, java.lang.String encoding)
+    // Template(java.lang.String name, java.lang.String sourceName, java.io.Reader reader, Configuration cfg, java.lang.String encoding)
+    exists(ConstructorCall cc, Expr e |
+      cc.getConstructor().getDeclaringType() instanceof TypeFreeMarkerTemplate and
+      e = cc.getAnArgument() and
+      (
+        e.getType().(RefType).hasQualifiedName("java.io", "Reader") and
+        this.asExpr() = e
+      )
+    )
+    or
+    exists(ConstructorCall cc |
+      cc.getConstructor().getDeclaringType() instanceof TypeFreeMarkerTemplate and
+      // Template(java.lang.String name, java.lang.String sourceCode, Configuration cfg)
+      cc.getNumArgument() = 3 and
+      cc.getArgument(1).getType() instanceof TypeString and
+      this.asExpr() = cc.getArgument(1)
+    )
+  }
+}
+
+/**
+ * An argument to FreeMarker template engine's `putTemplate` method call.
+ */
+private class FreeMarkerStringTemplateLoaderPutTemplateSink extends Sink {
+  FreeMarkerStringTemplateLoaderPutTemplateSink() {
+    exists(MethodAccess ma |
+      this.asExpr() = ma.getArgument(1) and
+      ma.getMethod() instanceof MethodFreeMarkerStringTemplateLoaderPutTemplate
+    )
+  }
+}
+
+/**
+ * An argument to Pebble template engine's `getLiteralTemplate` or `getTemplate` method call.
+ */
+private class PebbleGetTemplateSinkTemplateSink extends Sink {
+  PebbleGetTemplateSinkTemplateSink() {
+    exists(MethodAccess ma |
+      this.asExpr() = ma.getArgument(0) and
+      ma.getMethod() instanceof MethodPebbleGetTemplate
+    )
+  }
+}
+
+/**
+ * An argument to JinJava template engine's `render` or `renderForResult` method call.
+ */
+private class JinjavaRenderSink extends Sink {
+  JinjavaRenderSink() {
+    exists(MethodAccess ma |
+      this.asExpr() = ma.getArgument(0) and
+      (
+        ma.getMethod() instanceof MethodJinjavaRenderForResult
+        or
+        ma.getMethod() instanceof MethodJinjavaRender
+      )
+    )
+  }
+}
+
+/**
+ * An argument to ThymeLeaf template engine's `process` method call.
+ */
+private class ThymeLeafRenderSink extends Sink {
+  ThymeLeafRenderSink() {
+    exists(MethodAccess ma |
+      this.asExpr() = ma.getArgument(0) and
+      ma.getMethod() instanceof MethodThymeleafProcess
+    )
+  }
+}
+
+/**
+ * Tainted data flowing into a Velocity Context through `put` method taints the context.
+ */
+private class VelocityContextFlow extends AdditionalFlowStep {
+  override predicate isAdditionalTaintStep(DataFlow::Node prev, DataFlow::Node succ) {
+    exists(MethodAccess m | m.getMethod() instanceof MethodVelocityContextPut |
+      m.getArgument(1) = prev.asExpr() and
+      succ.asExpr() = m.getQualifier()
+    )
+  }
+}
+
+/**
+ * An argument to Velocity template engine's `mergeTemplate` method call.
+ */
+private class VelocityMergeTempSink extends Sink {
+  VelocityMergeTempSink() {
+    exists(MethodAccess m |
+      // static boolean	mergeTemplate(String templateName, String encoding, Context context, Writer writer)
+      m.getCallee() instanceof MethodVelocityMergeTemplate and
+      m.getArgument(2) = this.getExpr()
+    )
+  }
+}
+
+/**
+ * An argument to Velocity template engine's `mergeTemplate` method call.
+ */
+private class VelocityMergeSink extends Sink {
+  VelocityMergeSink() {
+    exists(MethodAccess m |
+      m.getCallee() instanceof MethodVelocityMerge and
+      // public void merge(Context context, Writer writer)
+      // public void merge(Context context, Writer writer, List<String> macroLibraries)
+      m.getArgument(0) = this.getExpr()
+    )
+  }
+}
+
+/**
+ * An argument to Velocity template engine's `evaluate` method call.
+ */
+private class VelocityEvaluateSink extends Sink {
+  VelocityEvaluateSink() {
+    exists(MethodAccess m |
+      m.getCallee() instanceof MethodVelocityEvaluate and
+      m.getArgument([0, 3]) = this.getExpr()
+    )
+  }
+}
+
+/**
+ * An argument to Velocity template engine's `parse` method call.
+ */
+private class VelocityParseSink extends Sink {
+  VelocityParseSink() {
+    exists(MethodAccess ma |
+      this.asExpr() = ma.getArgument(0) and
+      ma.getMethod() instanceof MethodVelocityParse
+    )
+  }
+}
+
+/**
+ * An argument to Velocity template engine's `putStringResource` method call.
+ */
+private class VelocityPutStringResSink extends Sink {
+  VelocityPutStringResSink() {
+    exists(MethodAccess ma |
+      this.asExpr() = ma.getArgument(1) and
+      ma.getMethod() instanceof MethodVelocityPutStringResource
+    )
+  }
+}
@@ -0,0 +1,29 @@
+/** Definitions related to the FreeMarker Templating library. */
+
+import java
+
+/** The `Template` class of the FreeMarker Template Engine */
+class TypeFreeMarkerTemplate extends Class {
+  TypeFreeMarkerTemplate() { this.hasQualifiedName("freemarker.template", "Template") }
+}
+
+/** The `process` method of the FreeMarker Template Engine's `Template` class */
+class MethodFreeMarkerTemplateProcess extends Method {
+  MethodFreeMarkerTemplateProcess() {
+    this.getDeclaringType() instanceof TypeFreeMarkerTemplate and
+    this.hasName("process")
+  }
+}
+
+/** The `StringTemplateLoader` class of the FreeMarker Template Engine */
+class TypeFreeMarkerStringLoader extends Class {
+  TypeFreeMarkerStringLoader() { this.hasQualifiedName("freemarker.cache", "StringTemplateLoader") }
+}
+
+/** The `process` method of the FreeMarker Template Engine's `StringTemplateLoader` class */
+class MethodFreeMarkerStringTemplateLoaderPutTemplate extends Method {
+  MethodFreeMarkerStringTemplateLoaderPutTemplate() {
+    this.getDeclaringType() instanceof TypeFreeMarkerStringLoader and
+    this.hasName("putTemplate")
+  }
+}
@@ -0,0 +1,24 @@
+/** Definitions related to the Jinjava Templating library. */
+
+import java
+
+/** The `Jinjava` class of the Jinjava Templating Engine. */
+class TypeJinjava extends Class {
+  TypeJinjava() { this.hasQualifiedName("com.hubspot.jinjava", "Jinjava") }
+}
+
+/** The `render` method of the Jinjava Templating Engine. */
+class MethodJinjavaRender extends Method {
+  MethodJinjavaRender() {
+    this.getDeclaringType() instanceof TypeJinjava and
+    this.hasName("render")
+  }
+}
+
+/** The `render` method of the Jinjava Templating Engine. */
+class MethodJinjavaRenderForResult extends Method {
+  MethodJinjavaRenderForResult() {
+    this.getDeclaringType() instanceof TypeJinjava and
+    this.hasName("renderForResult")
+  }
+}
@@ -0,0 +1,16 @@
+/** Definitions related to the Pebble Templating library. */
+
+import java
+
+/** The `PebbleEngine` class of the Pebble Templating Engine. */
+class TypePebbleEngine extends Class {
+  TypePebbleEngine() { this.hasQualifiedName("com.mitchellbosecke.pebble", "PebbleEngine") }
+}
+
+/** The `getTemplate` method of the Pebble Templating Engine. */
+class MethodPebbleGetTemplate extends Method {
+  MethodPebbleGetTemplate() {
+    this.getDeclaringType() instanceof TypePebbleEngine and
+    this.hasName(["getTemplate", "getLiteralTemplate"])
+  }
+}
@@ -0,0 +1,25 @@
+/** Definitions related to the Thymeleaf Templating library. */
+
+import java
+
+/**
+ * A class implementing the `ITemplateEngine` interface of the Thymeleaf
+ * Templating Engine such as the `TemplateEngine` class.
+ */
+class TypeThymeleafTemplateEngine extends Class {
+  TypeThymeleafTemplateEngine() {
+    this.hasQualifiedName("org.thymeleaf", "TemplateEngine")
+    or
+    exists(Type t | this.getASupertype*().extendsOrImplements(t) |
+      t.hasName("org.thymeleaf.ITemplateEngine")
+    )
+  }
+}
+
+/** The `process` or `processThrottled` method of the Thymeleaf Templating Engine. */
+class MethodThymeleafProcess extends Method {
+  MethodThymeleafProcess() {
+    this.getDeclaringType() instanceof TypeThymeleafTemplateEngine and
+    this.hasName(["process", "processThrottled"])
+  }
+}