HV-1816 Disable Expression Language by default for custom constraint violations

documentation/src/main/asciidoc/ch04.asciidoc

@@ -91,6 +91,19 @@ context:
 `format(String format, Object... args)` which behaves like
 `java.util.Formatter.format(String format, Object... args)`.
 
+Expression Language is very flexible and Hibernate Validator offers several feature levels
+that you can use to enable Expression Language features through the `ExpressionLanguageFeatureLevel` enum:
+
+* `NONE`: Expression Language interpolation is fully disabled.
+* `VARIABLES`: Allow interpolation of the variables injected via `addExpressionVariable()`, resources bundles and usage of the `formatter` object.
+* `BEAN_PROPERTIES`: Allow everything `VARIABLES` allows plus the interpolation of bean properties.
+* `BEAN_METHODS`: Also allow execution of bean methods. Can be considered safe for hardcoded constraint messages but not for <<section-hibernateconstraintvalidatorcontext, custom violations>>
+  where extra care is required.
+
+The default feature level for constraint messages is `BEAN_PROPERTIES`.
+
+You can define the Expression Language feature level when <<el-features, bootstrapping the `ValidatorFactory`>>.
+
 The following section provides several examples for using EL expressions in error messages.
 
 ==== Examples

documentation/src/main/asciidoc/ch06.asciidoc

@@ -173,35 +173,12 @@ It is important to add each configured constraint violation by calling `addConst
 Only after that the new constraint violation will be created.
 ====
 
-[[el-injection-caution]]
-[CAUTION]
-====
-**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 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 <<section-hibernateconstraintvalidatorcontext,pass it as an expression variable>>
-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:
+By default, Expression Language is not enabled for custom violations created in the `ConstraintValidatorContext`.
 
-[source]
-----
-include::{sourcedir}/org/hibernate/validator/referenceguide/chapter06/elinjection/SafeValidator.java[tags=include]
-----
+However, for some advanced requirements, using Expression Language might be necessary.
 
-By using expression variables, Hibernate Validator properly handles escaping and EL expressions won't be executed.
-====
+In this case, you need to unwrap the `HibernateConstraintValidatorContext` and enable Expression Language explicitly.
+See <<section-hibernateconstraintvalidatorcontext>> for more information.
 
 Refer to <<section-custom-property-paths>> to learn how to use the `ConstraintValidatorContext` API to
 control the property path of constraint violations for class-level constraints.

documentation/src/main/asciidoc/ch12.asciidoc

@@ -419,6 +419,55 @@ include::{sourcedir}/org/hibernate/validator/referenceguide/chapter12/dynamicpay
 ----
 ====
 
+[[el-features]]
+=== Enabling Expression Language features
+
+Hibernate Validator restricts the Expression Language features exposed by default.
+
+For this purpose, we define several feature levels in `ExpressionLanguageFeatureLevel`:
+
+* `NONE`: Expression Language interpolation is fully disabled.
+* `VARIABLES`: Allow interpolation of the variables injected via `addExpressionVariable()`, resources bundles and usage of the `formatter` object.
+* `BEAN_PROPERTIES`: Allow everything `VARIABLES` allows plus the interpolation of bean properties.
+* `BEAN_METHODS`: Also allow execution of bean methods. This can lead to serious security issues, including arbitrary code execution if not carefully handled.
+
+Depending on the context, the features we expose are different:
+
+* For constraints, the default level is `BEAN_PROPERTIES`.
+  For all the built-in constraint messages to be correctly interpolated, you need at least the `VARIABLES` level.
+* For custom violations, created via the `ConstraintValidatorContext`, Expression Language is disabled by default.
+  You can enable it for specific custom violations and, when enabled, it will default to `VARIABLES`.
+
+Hibernate Validator provides ways to override these defaults when boostrapping the `ValidatorFactory`.
+
+To change the Expression Language feature level for constraints, use the following:
+
+[source, JAVA, indent=0]
+----
+include::{sourcedir}/org/hibernate/validator/referenceguide/chapter12/el/ElFeaturesTest.java[tags=constraints]
+----
+
+To change the Expression Language feature level for custom violations, use the following:
+
+[source, JAVA, indent=0]
+----
+include::{sourcedir}/org/hibernate/validator/referenceguide/chapter12/el/ElFeaturesTest.java[tags=customViolations]
+----
+
+[CAUTION]
+====
+Doing this will automatically enable Expression Language for all the custom violations in your application.
+
+It should only be used for compatibility and to ease the migration from older Hibernate Validator versions.
+====
+
+These levels can also be defined using the following properties:
+
+* `hibernate.validator.constraint_expression_language_feature_level`
+* `hibernate.validator.custom_violation_expression_language_feature_level`
+
+Accepted values for these properties are: `none`, `variables`, `bean-properties` and `bean-methods`.
+
 [[non-el-message-interpolator]]
 === `ParameterMessageInterpolator`
 
@@ -510,6 +559,7 @@ custom extensions for both of these interfaces.
 [[section-custom-constraint-validator-context]]
 `HibernateConstraintValidatorContext` is a subtype of `ConstraintValidatorContext` which allows you to:
 
+* enable Expression Language interpolation for a particular custom violation - see below
 * set arbitrary parameters for interpolation via the Expression Language message interpolation
 facility using `HibernateConstraintValidatorContext#addExpressionVariable(String, Object)`
 or `HibernateConstraintValidatorContext#addMessageParameter(String, Object)`.
@@ -535,8 +585,8 @@ include::{sourcedir}/org/hibernate/validator/referenceguide/chapter12/context/My
 [NOTE]
 ====
 Apart from the syntax, the main difference between message parameters and expression variables is that message parameters
-are simply interpolated whereas expression variables are interpreted using the expression language engine.
-In practice, it should not change anything.
+are simply interpolated whereas expression variables are interpreted using the Expression Language engine.
+In practice, use message parameters if you do not need the advanced features of an Expression Language.
 ====
 +
 [NOTE]
@@ -550,6 +600,50 @@ You can, however, update the parameters between invocations of
 ====
 * set an arbitrary dynamic payload - see <<section-dynamic-payload>>
 
+By default, Expression Language interpolation is **disabled** for custom violations,
+this to avoid arbitrary code execution or sensitive data leak if message templates are built from improperly escaped user input.
+
+It is possible to enable Expression Language for a given custom violation by using `enableExpressionLanguage()` as shown in the example below:
+
+[source]
+----
+include::{sourcedir}/org/hibernate/validator/referenceguide/chapter06/elinjection/SafeValidator.java[tags=include]
+----
+
+In this case, the message template will be interpolated by the Expression Language engine.
+
+By default, only variables interpolation is enabled when enabling Expression Language.
+
+You can enable more features by using `HibernateConstraintViolationBuilder#enableExpressionLanguage(ExpressionLanguageFeatureLevel level)`.
+
+We define several levels of features for Expression Language interpolation:
+
+* `NONE`: Expression Language interpolation is fully disabled - this is the default for custom violations.
+* `VARIABLES`: Allow interpolation of the variables injected via `addExpressionVariable()`, resources bundles and usage of the `formatter` object.
+* `BEAN_PROPERTIES`: Allow everything `VARIABLES` allows plus the interpolation of bean properties.
+* `BEAN_METHODS`: Also allow execution of bean methods. This can lead to serious security issues, including arbitrary code execution if not carefully handled.
+
+[CAUTION]
+====
+Using `addExpressionVariable()` is the only safe way to inject a variable into an expression
+and it's especially important if you use the `BEAN_PROPERTIES` or `BEAN_METHODS` feature levels.
+
+If you inject user input by simply concatenating the user input in the message,
+you will allow potential arbitrary code execution and sensitive data leak:
+if the user input contains valid expressions, they will be executed by the Expression Language engine.
+
+Here is an example of something you should **ABSOLUTELY NOT** do:
+
+[source, JAVA, indent=0]
+----
+include::{sourcedir}/org/hibernate/validator/referenceguide/chapter06/elinjection/UnsafeValidator.java[tags=include]
+----
+
+In the example above, if `value`, which might be user input, contains a valid expression,
+it will be interpolated by the Expression Language engine,
+potentially leading to unsafe behaviors.
+====
+
 ==== `HibernateMessageInterpolatorContext`
 
 Hibernate Validator also offers a custom extension of `MessageInterpolatorContext`, namely
@@ -563,7 +657,7 @@ bundle. If you have any other use cases, let us know.
 ====
 [source, JAVA, indent=0]
 ----
-include::{engine-sourcedir}/org/hibernate/validator/messageinterpolation/HibernateMessageInterpolatorContext.java[lines=18..26]
+include::{engine-sourcedir}/org/hibernate/validator/messageinterpolation/HibernateMessageInterpolatorContext.java[lines=22..58]
 ----
 ====
 

documentation/src/test/java/org/hibernate/validator/referenceguide/chapter06/elinjection/SafeValidator.java

@@ -23,6 +23,7 @@ public boolean isValid(String value, ConstraintValidatorContext context) {
 			hibernateContext
 					.addExpressionVariable( "validatedValue", value )
 					.buildConstraintViolationWithTemplate( "${validatedValue} is not a valid ZIP code" )
+					.enableExpressionLanguage()
 					.addConstraintViolation();
 
 			return false;

documentation/src/test/java/org/hibernate/validator/referenceguide/chapter06/elinjection/UnsafeValidator.java

@@ -1,5 +1,6 @@
 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;
@@ -16,9 +17,15 @@ public boolean isValid(String value, ConstraintValidatorContext context) {
 
 		context.disableDefaultConstraintViolation();
 
+		HibernateConstraintValidatorContext hibernateContext = context.unwrap(
+				HibernateConstraintValidatorContext.class );
+		hibernateContext.disableDefaultConstraintViolation();
+
 		if ( isInvalid( value ) ) {
-			context
+			hibernateContext
+					// THIS IS UNSAFE, DO NOT COPY THIS EXAMPLE
 					.buildConstraintViolationWithTemplate( value + " is not a valid ZIP code" )
+					.enableExpressionLanguage()
 					.addConstraintViolation();
 
 			return false;

documentation/src/test/java/org/hibernate/validator/referenceguide/chapter12/el/ElFeaturesTest.java

@@ -0,0 +1,33 @@
+package org.hibernate.validator.referenceguide.chapter12.el;
+
+import javax.validation.Validation;
+import javax.validation.ValidatorFactory;
+
+import org.hibernate.validator.HibernateValidator;
+import org.hibernate.validator.messageinterpolation.ExpressionLanguageFeatureLevel;
+import org.junit.Test;
+
+public class ElFeaturesTest {
+
+	@SuppressWarnings("unused")
+	@Test
+	public void testConstraints() throws Exception {
+		//tag::constraints[]
+		ValidatorFactory validatorFactory = Validation.byProvider( HibernateValidator.class )
+				.configure()
+				.constraintExpressionLanguageFeatureLevel( ExpressionLanguageFeatureLevel.VARIABLES )
+				.buildValidatorFactory();
+		//end::constraints[]
+	}
+
+	@SuppressWarnings("unused")
+	@Test
+	public void testCustomViolations() throws Exception {
+		//tag::customViolations[]
+		ValidatorFactory validatorFactory = Validation.byProvider( HibernateValidator.class )
+				.configure()
+				.customViolationExpressionLanguageFeatureLevel( ExpressionLanguageFeatureLevel.VARIABLES )
+				.buildValidatorFactory();
+		//end::customViolations[]
+	}
+}

engine/src/main/java/org/hibernate/validator/BaseHibernateValidatorConfiguration.java

@@ -13,6 +13,7 @@
 import java.util.Set;
 
 import javax.validation.Configuration;
+import javax.validation.ConstraintValidatorContext;
 import javax.validation.ConstraintViolation;
 import javax.validation.TraversableResolver;
 import javax.validation.constraints.Future;
@@ -24,6 +25,7 @@
 import org.hibernate.validator.cfg.ConstraintMapping;
 import org.hibernate.validator.constraints.ParameterScriptAssert;
 import org.hibernate.validator.constraints.ScriptAssert;
+import org.hibernate.validator.messageinterpolation.ExpressionLanguageFeatureLevel;
 import org.hibernate.validator.spi.messageinterpolation.LocaleResolver;
 import org.hibernate.validator.metadata.BeanMetaDataClassNormalizer;
 import org.hibernate.validator.spi.nodenameprovider.PropertyNodeNameProvider;
@@ -144,6 +146,28 @@ public interface BaseHibernateValidatorConfiguration<S extends BaseHibernateVali
 	@Incubating
 	String LOCALE_RESOLVER_CLASSNAME = "hibernate.validator.locale_resolver";
 
+	/**
+	 * Property for configuring the Expression Language feature level for constraints, allowing to define which
+	 * Expression Language features are available for message interpolation.
+	 * <p>
+	 * This property only affects the EL feature level of "static" constraint violation messages. In particular, it
+	 * doesn't affect the default EL feature level for custom violations. Refer to
+	 * {@link #CUSTOM_VIOLATION_EXPRESSION_LANGUAGE_FEATURE_LEVEL} to configure that.
+	 *
+	 * @since 6.2
+	 */
+	@Incubating
+	String CONSTRAINT_EXPRESSION_LANGUAGE_FEATURE_LEVEL = "hibernate.validator.constraint_expression_language_feature_level";
+
+	/**
+	 * Property for configuring the Expression Language feature level for custom violations, allowing to define which
+	 * Expression Language features are available for message interpolation.
+	 *
+	 * @since 6.2
+	 */
+	@Incubating
+	String CUSTOM_VIOLATION_EXPRESSION_LANGUAGE_FEATURE_LEVEL = "hibernate.validator.custom_violation_expression_language_feature_level";
+
 	/**
 	 * <p>
 	 * Returns the {@link ResourceBundleLocator} used by the
@@ -427,4 +451,33 @@ default S locales(Locale... locales) {
 
 	@Incubating
 	S beanMetaDataClassNormalizer(BeanMetaDataClassNormalizer beanMetaDataClassNormalizer);
+
+	/**
+	 * Allows setting the Expression Language feature level for message interpolation of constraint messages.
+	 * <p>
+	 * This is the feature level used for messages hardcoded inside the constraint declaration.
+	 * <p>
+	 * If you are creating custom constraint violations, Expression Language support needs to be explicitly enabled and
+	 * use the safest feature level by default if enabled.
+	 *
+	 * @param expressionLanguageFeatureLevel the {@link ExpressionLanguageFeatureLevel} to be used
+	 * @return {@code this} following the chaining method pattern
+	 *
+	 * @since 6.2
+	 */
+	@Incubating
+	S constraintExpressionLanguageFeatureLevel(ExpressionLanguageFeatureLevel expressionLanguageFeatureLevel);
+
+	/**
+	 * Allows setting the Expression Language feature level for message interpolation of custom violation messages.
+	 * <p>
+	 * This is the feature level used for messages of custom violations created by the {@link ConstraintValidatorContext}.
+	 *
+	 * @param expressionLanguageFeatureLevel the {@link ExpressionLanguageFeatureLevel} to be used
+	 * @return {@code this} following the chaining method pattern
+	 *
+	 * @since 6.2
+	 */
+	@Incubating
+	S customViolationExpressionLanguageFeatureLevel(ExpressionLanguageFeatureLevel expressionLanguageFeatureLevel);
 }

engine/src/main/java/org/hibernate/validator/constraintvalidation/HibernateConstraintValidatorContext.java

@@ -21,6 +21,9 @@
  */
 public interface HibernateConstraintValidatorContext extends ConstraintValidatorContext {
 
+	@Override
+	HibernateConstraintViolationBuilder buildConstraintViolationWithTemplate(String messageTemplate);
+
 	/**
 	 * Allows to set an additional named parameter which can be interpolated in the constraint violation message. The
 	 * variable will be available for interpolation for all constraint violations generated for this constraint.

engine/src/main/java/org/hibernate/validator/constraintvalidation/HibernateConstraintViolationBuilder.java

@@ -0,0 +1,43 @@
+/*
+ * Hibernate Validator, declare and validate application constraints
+ *
+ * License: Apache License, Version 2.0
+ * See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>.
+ */
+
+package org.hibernate.validator.constraintvalidation;
+
+import javax.validation.ConstraintValidatorContext.ConstraintViolationBuilder;
+
+import org.hibernate.validator.Incubating;
+import org.hibernate.validator.messageinterpolation.ExpressionLanguageFeatureLevel;
+
+public interface HibernateConstraintViolationBuilder extends ConstraintViolationBuilder {
+
+	/**
+	 * Enable Expression Language with the default Expression Language feature level for the constraint violation
+	 * created by this builder if the chosen {@code MessageInterpolator} supports it.
+	 * <p>
+	 * If you enable this, you need to make sure your message template does not contain any unescaped user input (such as
+	 * the validated value): use {@code addExpressionVariable()} to inject properly escaped variables into the template.
+	 *
+	 * @since 6.2
+	 */
+	@Incubating
+	default HibernateConstraintViolationBuilder enableExpressionLanguage() {
+		return enableExpressionLanguage( ExpressionLanguageFeatureLevel.DEFAULT );
+	};
+
+	/**
+	 * Enable Expression Language for the constraint violation created by this builder if the chosen
+	 * {@code MessageInterpolator} supports it.
+	 * <p>
+	 * If you enable this, you need to make sure your message template does not contain any unescaped user input (such as
+	 * the validated value): use {@code addExpressionVariable()} to inject properly escaped variables into the template.
+	 *
+	 * @param level The Expression Language features level supported.
+	 * @since 6.2
+	 */
+	@Incubating
+	HibernateConstraintViolationBuilder enableExpressionLanguage(ExpressionLanguageFeatureLevel level);
+}