Skip to content

Commit cbb64cc

Browse files
atorralbaatorralba
authored
Merge pull request #10352 from atorralba/atorralba/promote-template-injection
Java: Promote Server-side template injection from experimental
2 parents 4614074 + 4af29e6 commit cbb64cc

File tree

615 files changed

+16783
-702
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

615 files changed

+16783
-702
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ private module Frameworks {
117117
private import semmle.code.java.frameworks.Retrofit
118118
private import semmle.code.java.frameworks.Stream
119119
private import semmle.code.java.frameworks.Strings
120+
private import semmle.code.java.frameworks.Thymeleaf
120121
private import semmle.code.java.frameworks.ratpack.Ratpack
121122
private import semmle.code.java.frameworks.ratpack.RatpackExec
122123
private import semmle.code.java.frameworks.spring.SpringCache
@@ -141,6 +142,7 @@ private module Frameworks {
141142
private import semmle.code.java.security.LdapInjection
142143
private import semmle.code.java.security.MvelInjection
143144
private import semmle.code.java.security.OgnlInjection
145+
private import semmle.code.java.security.TemplateInjection
144146
private import semmle.code.java.security.XPath
145147
private import semmle.code.java.security.XsltInjection
146148
private import semmle.code.java.frameworks.Jdbc
@@ -625,7 +627,7 @@ module CsvValidation {
625627
"open-url", "jndi-injection", "ldap", "sql", "jdbc-url", "logging", "mvel", "xpath",
626628
"groovy", "xss", "ognl-injection", "intent-start", "pending-intent-sent",
627629
"url-open-stream", "url-redirect", "create-file", "write-file", "set-hostname-verifier",
628-
"header-splitting", "information-leak", "xslt", "jexl", "bean-validation"
630+
"header-splitting", "information-leak", "xslt", "jexl", "bean-validation", "ssti"
629631
] and
630632
not kind.matches("regex-use%") and
631633
not kind.matches("qltest%") and

‎java/ql/lib/semmle/code/java/frameworks/Thymeleaf.qll‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/**
2+
* Provides classes and predicates for working with the Thymeleaf template engine.
3+
*/
4+
5+
import java
6+
private import semmle.code.java.dataflow.ExternalFlow
7+
8+
private class ThymeleafSummaryModels extends SummaryModelCsv {
9+
override predicate row(string row) {
10+
row =
11+
[
12+
"org.thymeleaf;TemplateSpec;false;TemplateSpec;;;Argument[0];Argument[-1];taint;manual",
13+
"org.thymeleaf;TemplateSpec;false;getTemplate;;;Argument[-1];ReturnValue;taint;manual",
14+
]
15+
}
16+
}

‎java/ql/lib/semmle/code/java/security/TemplateInjection.qll‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/** Definitions related to the server-side template injection (SST) query. */
2+
3+
import java
4+
private import semmle.code.java.dataflow.FlowSources
5+
private import semmle.code.java.dataflow.ExternalFlow
6+
private import semmle.code.java.dataflow.TaintTracking
7+
8+
/**
9+
* A source for server-side template injection (SST) vulnerabilities.
10+
*/
11+
abstract class TemplateInjectionSource extends DataFlow::Node {
12+
/** Holds if this source has the specified `state`. */
13+
predicate hasState(DataFlow::FlowState state) { state instanceof DataFlow::FlowStateEmpty }
14+
}
15+
16+
/**
17+
* A sink for server-side template injection (SST) vulnerabilities.
18+
*/
19+
abstract class TemplateInjectionSink extends DataFlow::Node {
20+
/** Holds if this sink has the specified `state`. */
21+
predicate hasState(DataFlow::FlowState state) { state instanceof DataFlow::FlowStateEmpty }
22+
}
23+
24+
/**
25+
* A unit class for adding additional taint steps.
26+
*
27+
* Extend this class to add additional taint steps that should apply to flows related to
28+
* server-side template injection (SST) vulnerabilities.
29+
*/
30+
class TemplateInjectionAdditionalTaintStep extends Unit {
31+
/**
32+
* Holds if the step from `node1` to `node2` should be considered a taint
33+
* step for flows related to server-side template injection (SST) vulnerabilities.
34+
*/
35+
predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() }
36+
37+
/**
38+
* Holds if the step from `node1` to `node2` should be considered a taint
39+
* step for flows related toserver-side template injection (SST) vulnerabilities.
40+
* This step is only applicable in `state1` and updates the flow state to `state2`.
41+
*/
42+
predicate isAdditionalTaintStep(
43+
DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
44+
DataFlow::FlowState state2
45+
) {
46+
none()
47+
}
48+
}
49+
50+
/**
51+
* A sanitizer for server-side template injection (SST) vulnerabilities.
52+
*/
53+
abstract class TemplateInjectionSanitizer extends DataFlow::Node { }
54+
55+
/**
56+
* A sanitizer for server-side template injection (SST) vulnerabilities.
57+
* This sanitizer is only applicable when `TemplateInjectionSanitizerWithState::hasState`
58+
* holds for the flow state.
59+
*/
60+
abstract class TemplateInjectionSanitizerWithState extends DataFlow::Node {
61+
/** Holds if this sanitizer has the specified `state`. */
62+
abstract predicate hasState(DataFlow::FlowState state);
63+
}
64+
65+
private class DefaultTemplateInjectionSource extends TemplateInjectionSource instanceof RemoteFlowSource {
66+
}
67+
68+
private class DefaultTemplateInjectionSink extends TemplateInjectionSink {
69+
DefaultTemplateInjectionSink() { sinkNode(this, "ssti") }
70+
}
71+
72+
private class DefaultTemplateInjectionSanitizer extends TemplateInjectionSanitizer {
73+
DefaultTemplateInjectionSanitizer() {
74+
this.getType() instanceof PrimitiveType or
75+
this.getType() instanceof BoxedType or
76+
this.getType() instanceof NumericType
77+
}
78+
}
79+
80+
private class TemplateInjectionSinkModels extends SinkModelCsv {
81+
override predicate row(string row) {
82+
row =
83+
[
84+
"freemarker.template;Template;true;Template;(String,Reader);;Argument[1];ssti;manual",
85+
"freemarker.template;Template;true;Template;(String,Reader,Configuration);;Argument[1];ssti;manual",
86+
"freemarker.template;Template;true;Template;(String,Reader,Configuration,String);;Argument[1];ssti;manual",
87+
"freemarker.template;Template;true;Template;(String,String,Reader,Configuration);;Argument[2];ssti;manual",
88+
"freemarker.template;Template;true;Template;(String,String,Reader,Configuration,String);;Argument[2];ssti;manual",
89+
"freemarker.template;Template;true;Template;(String,String,Reader,Configuration,ParserConfiguration,String);;Argument[2];ssti;manual",
90+
"freemarker.template;Template;true;Template;(String,String,Configuration);;Argument[1];ssti;manual",
91+
"freemarker.cache;StringTemplateLoader;true;putTemplate;;;Argument[1];ssti;manual",
92+
"com.mitchellbosecke.pebble;PebbleEngine;true;getTemplate;;;Argument[0];ssti;manual",
93+
"com.mitchellbosecke.pebble;PebbleEngine;true;getLiteralTemplate;;;Argument[0];ssti;manual",
94+
"com.hubspot.jinjava;Jinjava;true;renderForResult;;;Argument[0];ssti;manual",
95+
"com.hubspot.jinjava;Jinjava;true;render;;;Argument[0];ssti;manual",
96+
"org.thymeleaf;ITemplateEngine;true;process;;;Argument[0];ssti;manual",
97+
"org.thymeleaf;ITemplateEngine;true;processThrottled;;;Argument[0];ssti;manual",
98+
"org.apache.velocity.app;Velocity;true;evaluate;;;Argument[3];ssti;manual",
99+
"org.apache.velocity.app;Velocity;true;mergeTemplate;;;Argument[2];ssti;manual",
100+
"org.apache.velocity.app;VelocityEngine;true;evaluate;;;Argument[3];ssti;manual",
101+
"org.apache.velocity.app;VelocityEngine;true;mergeTemplate;;;Argument[2];ssti;manual",
102+
"org.apache.velocity.runtime.resource.util;StringResourceRepository;true;putStringResource;;;Argument[1];ssti;manual",
103+
"org.apache.velocity.runtime;RuntimeServices;true;evaluate;;;Argument[3];ssti;manual",
104+
"org.apache.velocity.runtime;RuntimeServices;true;parse;;;Argument[0];ssti;manual",
105+
"org.apache.velocity.runtime;RuntimeSingleton;true;parse;;;Argument[0];ssti;manual"
106+
]
107+
}
108+
}

‎java/ql/lib/semmle/code/java/security/TemplateInjectionQuery.qll‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/** Provides a taint tracking configuration for server-side template injection (SST) vulnerabilities */
2+
3+
import java
4+
import semmle.code.java.dataflow.TaintTracking
5+
import semmle.code.java.dataflow.FlowSources
6+
import semmle.code.java.security.TemplateInjection
7+
8+
/** A taint tracking configuration to reason about server-side template injection (SST) vulnerabilities */
9+
class TemplateInjectionFlowConfig extends TaintTracking::Configuration {
10+
TemplateInjectionFlowConfig() { this = "TemplateInjectionFlowConfig" }
11+
12+
override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
13+
source.(TemplateInjectionSource).hasState(state)
14+
}
15+
16+
override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
17+
sink.(TemplateInjectionSink).hasState(state)
18+
}
19+
20+
override predicate isSanitizer(DataFlow::Node sanitizer) {
21+
sanitizer instanceof TemplateInjectionSanitizer
22+
}
23+
24+
override predicate isSanitizer(DataFlow::Node sanitizer, DataFlow::FlowState state) {
25+
sanitizer.(TemplateInjectionSanitizerWithState).hasState(state)
26+
}
27+
28+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
29+
any(TemplateInjectionAdditionalTaintStep a).isAdditionalTaintStep(node1, node2)
30+
}
31+
32+
override predicate isAdditionalTaintStep(
33+
DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
34+
DataFlow::FlowState state2
35+
) {
36+
any(TemplateInjectionAdditionalTaintStep a).isAdditionalTaintStep(node1, state1, node2, state2)
37+
}
38+
}

java/ql/src/experimental/Security/CWE/CWE-094/SSTIBad.java renamed to java/ql/src/Security/CWE/CWE-094/SSTIBad.java

File renamed without changes.

java/ql/src/experimental/Security/CWE/CWE-094/SSTIGood.java renamed to java/ql/src/Security/CWE/CWE-094/SSTIGood.java

File renamed without changes.

‎java/ql/src/Security/CWE/CWE-094/TemplateInjection.qhelp‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
<p>
5+
Template injection occurs when user input is embedded in a template's code in an unsafe manner.
6+
An attacker can use native template syntax to inject a malicious payload into a template, which is then executed server-side.
7+
This permits the attacker to run arbitrary code in the server's context.
8+
</p>
9+
</overview>
10+
<recommendation>
11+
<p>
12+
To fix this, ensure that untrusted input is not used as part of a template's code. If the application requirements do not allow this,
13+
use a sandboxed environment where access to unsafe attributes and methods is prohibited.
14+
</p>
15+
</recommendation>
16+
<example>
17+
<p>
18+
In the example given below, an untrusted HTTP parameter <code>code</code> is used as a Velocity template string.
19+
This can lead to remote code execution.
20+
</p>
21+
<sample src="SSTIBad.java" />
22+
23+
<p>
24+
In the next example, the problem is avoided by using a fixed template string <code>s</code>.
25+
Since the template's code is not attacker-controlled in this case, this solution prevents the execution of untrusted code.
26+
</p>
27+
<sample src="SSTIGood.java" />
28+
</example>
29+
<references>
30+
<li>Portswigger: <a href="https://portswigger.net/web-security/server-side-template-injection">Server Side Template Injection</a>.</li>
31+
</references>
32+
</qhelp>

java/ql/src/experimental/Security/CWE/CWE-094/TemplateInjection.ql renamed to java/ql/src/Security/CWE/CWE-094/TemplateInjection.ql

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,18 @@
11
/**
2-
* @name Server Side Template Injection
3-
* @description Untrusted input used as a template parameter can lead to remote code execution.
2+
* @name Server-side template injection
3+
* @description Untrusted input interpreted as a template can lead to remote code execution.
44
* @kind path-problem
55
* @problem.severity error
6+
* @security-severity 9.3
67
* @precision high
78
* @id java/server-side-template-injection
89
* @tags security
10+
* external/cwe/cwe-1336
911
* external/cwe/cwe-094
1012
*/
1113

1214
import java
13-
import TemplateInjection
15+
import semmle.code.java.security.TemplateInjectionQuery
1416
import DataFlow::PathGraph
1517

1618
from TemplateInjectionFlowConfig config, DataFlow::PathNode source, DataFlow::PathNode sink

‎java/ql/src/change-notes/2022-09-08-server-side-template-injection-query.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* The query "Server-side template injection" (`java/server-side-template-injection`) has been promoted from experimental to the main query pack. This query was originally [submitted as an experimental query by @porcupineyhairs](https://github.com/github/codeql/pull/5935).

‎java/ql/src/experimental/Security/CWE/CWE-094/TemplateInjection.qhelp‎

Lines changed: 0 additions & 31 deletions
This file was deleted.

0 commit comments

Comments
 (0)