From 5903f446b6b2eda041e5ebbd5ce6ca8123bbfe99 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Thu, 30 Apr 2020 16:17:10 +0200 Subject: [PATCH] HV-1773 Be more explicit about issues with EL injection and how to avoid them --- documentation/src/main/asciidoc/ch06.asciidoc | 30 ++++++++++---- .../chapter06/elinjection/SafeValidator.java | 39 +++++++++++++++++++ .../elinjection/UnsafeValidator.java | 35 +++++++++++++++++ 3 files changed, 97 insertions(+), 7 deletions(-) create mode 100644 documentation/src/test/java/org/hibernate/validator/referenceguide/chapter06/elinjection/SafeValidator.java create mode 100644 documentation/src/test/java/org/hibernate/validator/referenceguide/chapter06/elinjection/UnsafeValidator.java diff --git a/documentation/src/main/asciidoc/ch06.asciidoc b/documentation/src/main/asciidoc/ch06.asciidoc index afb53c4b5a..18d93ff26f 100644 --- a/documentation/src/main/asciidoc/ch06.asciidoc +++ b/documentation/src/main/asciidoc/ch06.asciidoc @@ -173,18 +173,34 @@ It is important to add each configured constraint violation by calling `addConst Only after that the new constraint violation will be created. ==== -[WARNING] +[[el-injection-caution]] +[CAUTION] ==== -Note that the custom message template is passed directly to the Expression Language engine. +**Be aware that the custom message template is passed directly to the Expression Language engine.** Thus, you should be very careful when integrating user input in a custom message template as it will be interpreted -by the Expression Language engine, which is usually not the behavior you expect and could allow malicious users to leak -sensitive data. +by the Expression Language engine, which is usually not the behavior you want and **could allow malicious users to leak +sensitive data or even execute arbitrary code**. + +If you need to integrate user input in your message, you must <> +by unwrapping the context to `HibernateConstraintValidatorContext`. + +The following validator is very unsafe as it includes user input in the violation message. +If the validated `value` contains EL expressions, they will be executed by the EL engine. + +[source, JAVA, indent=0] +---- +include::{sourcedir}/org/hibernate/validator/referenceguide/chapter06/elinjection/UnsafeValidator.java[tags=include] +---- + +The following pattern must be used instead: -If you need to integrate user input, you should: +[source] +---- +include::{sourcedir}/org/hibernate/validator/referenceguide/chapter06/elinjection/SafeValidator.java[tags=include] +---- - * either escape it by using the http://beanvalidation.org/2.0/spec/#validationapi-message-defaultmessageinterpolation[Bean Validation message interpolation escaping rules]; - * or, even better, <> by unwrapping the context to `HibernateConstraintValidatorContext`. +By using expression variables, Hibernate Validator properly handles escaping and EL expressions won't be executed. ==== Refer to <> to learn how to use the `ConstraintValidatorContext` API to diff --git a/documentation/src/test/java/org/hibernate/validator/referenceguide/chapter06/elinjection/SafeValidator.java b/documentation/src/test/java/org/hibernate/validator/referenceguide/chapter06/elinjection/SafeValidator.java new file mode 100644 index 0000000000..182ab328da --- /dev/null +++ b/documentation/src/test/java/org/hibernate/validator/referenceguide/chapter06/elinjection/SafeValidator.java @@ -0,0 +1,39 @@ +package org.hibernate.validator.referenceguide.chapter06.elinjection; + +import org.hibernate.validator.constraintvalidation.HibernateConstraintValidatorContext; +import org.hibernate.validator.referenceguide.chapter06.constraintvalidatorpayload.ZipCode; + +import javax.validation.ConstraintValidator; +import javax.validation.ConstraintValidatorContext; + +//tag::include[] +public class SafeValidator implements ConstraintValidator { + + @Override + public boolean isValid(String value, ConstraintValidatorContext context) { + if ( value == null ) { + return true; + } + + HibernateConstraintValidatorContext hibernateContext = context.unwrap( + HibernateConstraintValidatorContext.class ); + hibernateContext.disableDefaultConstraintViolation(); + + if ( isInvalid( value ) ) { + hibernateContext + .addExpressionVariable( "validatedValue", value ) + .buildConstraintViolationWithTemplate( "${validatedValue} is not a valid ZIP code" ) + .addConstraintViolation(); + + return false; + } + + return true; + } + + private boolean isInvalid(String value) { + // ... + return false; + } +} +// end::include[] diff --git a/documentation/src/test/java/org/hibernate/validator/referenceguide/chapter06/elinjection/UnsafeValidator.java b/documentation/src/test/java/org/hibernate/validator/referenceguide/chapter06/elinjection/UnsafeValidator.java new file mode 100644 index 0000000000..6adf4632a1 --- /dev/null +++ b/documentation/src/test/java/org/hibernate/validator/referenceguide/chapter06/elinjection/UnsafeValidator.java @@ -0,0 +1,35 @@ +package org.hibernate.validator.referenceguide.chapter06.elinjection; + +import org.hibernate.validator.referenceguide.chapter06.constraintvalidatorpayload.ZipCode; + +import javax.validation.ConstraintValidator; +import javax.validation.ConstraintValidatorContext; + +//tag::include[] +public class UnsafeValidator implements ConstraintValidator { + + @Override + public boolean isValid(String value, ConstraintValidatorContext context) { + if ( value == null ) { + return true; + } + + context.disableDefaultConstraintViolation(); + + if ( isInvalid( value ) ) { + context + .buildConstraintViolationWithTemplate( value + " is not a valid ZIP code" ) + .addConstraintViolation(); + + return false; + } + + return true; + } + + private boolean isInvalid(String value) { + // ... + return false; + } +} +// end::include[]