address review comments

This commit is contained in:
Alvaro Muñoz
2020-10-22 14:14:48 +02:00
committed by Alvaro Muñoz
parent 73fc9fda77
commit 27bd9044e7
2 changed files with 4 additions and 18 deletions

View File

@@ -4,7 +4,9 @@
<qhelp>
<overview>
<p>When building custom constraint violation error messages, it is important to understand that they support different types of interpolation, including [Java EL expressions](https://docs.jboss.org/hibernate/validator/5.1/reference/en-US/html/chapter-message-interpolation.html#section-interpolation-with-message-expressions). Therefore if an attacker can inject arbitrary data in the error message template being passed to `ConstraintValidatorContext.buildConstraintViolationWithTemplate()` argument, he will be able to run arbitrary Java code. Unfortunately, it is common that validated (and therefore, normally untrusted) bean properties flow into the custom error message.</p>
<p>Bean validation custom constraint error messages support different types of interpolation, including [Java EL expressions](https://docs.jboss.org/hibernate/validator/5.1/reference/en-US/html/chapter-message-interpolation.html#section-interpolation-with-message-expressions).
Controlling part of the error message template being passed to `ConstraintValidatorContext.buildConstraintViolationWithTemplate()` argument will lead to arbitrary Java code execution.
Unfortunately, it is common that validated (and therefore, normally untrusted) bean properties flow into the custom error message.</p>
</overview>
<recommendation>
@@ -35,5 +37,6 @@ Validator validator = Validation.byDefaultProvider()
<references>
<li>https://docs.jboss.org/hibernate/stable/validator/reference/en-US/html_single/#_the_code_constraintvalidatorcontext_code</li>
<li>https://securitylab.github.com/research/bean-validation-RCE</li>
</references>
</qhelp>

View File

@@ -32,23 +32,6 @@ class BeanValidationSource extends RemoteFlowSource {
override string getSourceType() { result = "BeanValidation source" }
}
class ExceptionTaintStep extends TaintTracking::AdditionalTaintStep {
override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
exists(Call call, TryStmt t, CatchClause c, MethodAccess gm |
call.getEnclosingStmt().getEnclosingStmt*() = t.getBlock() and
t.getACatchClause() = c and
(
call.getCallee().getAThrownExceptionType().getASubtype*() = c.getACaughtType() or
c.getACaughtType().getASupertype*() instanceof TypeRuntimeException
) and
c.getVariable().getAnAccess() = gm.getQualifier() and
gm.getMethod().getName().regexpMatch("get(Localized)?Message|toString") and
n1.asExpr() = call.getAnArgument() and
n2.asExpr() = gm
)
}
}
class BuildConstraintViolationWithTemplateMethod extends Method {
BuildConstraintViolationWithTemplateMethod() {
getDeclaringType()