HV-1816 Limit the EL features exposed by default

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/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`
 
@@ -552,7 +601,7 @@ 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 user input is not properly escaped.
+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:
 
@@ -563,9 +612,21 @@ include::{sourcedir}/org/hibernate/validator/referenceguide/chapter06/elinjectio
 
 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.
+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:
@@ -596,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/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/HibernateConstraintViolationBuilder.java

@@ -10,18 +10,34 @@
 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 enabling this, you need to make sure your message template does not contain any unescaped user input (such as
+	 * 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();
+	HibernateConstraintViolationBuilder enableExpressionLanguage(ExpressionLanguageFeatureLevel level);
 }

engine/src/main/java/org/hibernate/validator/internal/engine/AbstractConfigurationImpl.java

@@ -57,6 +57,7 @@
 import org.hibernate.validator.internal.util.stereotypes.Lazy;
 import org.hibernate.validator.internal.xml.config.ValidationBootstrapParameters;
 import org.hibernate.validator.internal.xml.config.ValidationXmlParser;
+import org.hibernate.validator.messageinterpolation.ExpressionLanguageFeatureLevel;
 import org.hibernate.validator.messageinterpolation.ResourceBundleMessageInterpolator;
 import org.hibernate.validator.metadata.BeanMetaDataClassNormalizer;
 import org.hibernate.validator.resourceloading.PlatformResourceBundleLocator;
@@ -129,6 +130,8 @@ public abstract class AbstractConfigurationImpl<T extends BaseHibernateValidator
 	private Locale defaultLocale = Locale.getDefault();
 	private LocaleResolver localeResolver;
 	private BeanMetaDataClassNormalizer beanMetaDataClassNormalizer;
+	private ExpressionLanguageFeatureLevel constraintExpressionLanguageFeatureLevel;
+	private ExpressionLanguageFeatureLevel customViolationExpressionLanguageFeatureLevel;
 
 	protected AbstractConfigurationImpl(BootstrapState state) {
 		this();
@@ -627,6 +630,35 @@ public BeanMetaDataClassNormalizer getBeanMetaDataClassNormalizer() {
 		return beanMetaDataClassNormalizer;
 	}
 
+	@Override
+	public T constraintExpressionLanguageFeatureLevel(ExpressionLanguageFeatureLevel expressionLanguageFeatureLevel) {
+		if ( LOG.isDebugEnabled() ) {
+			if ( expressionLanguageFeatureLevel != null ) {
+				LOG.debug( "Setting ExpressionLanguageFeatureLevel for constraints to " + expressionLanguageFeatureLevel.name() );
+			}
+		}
+		this.constraintExpressionLanguageFeatureLevel = expressionLanguageFeatureLevel;
+		return thisAsT();
+	}
+
+	public ExpressionLanguageFeatureLevel getConstraintExpressionLanguageFeatureLevel() {
+		return constraintExpressionLanguageFeatureLevel;
+	}
+
+	@Override
+	public T customViolationExpressionLanguageFeatureLevel(ExpressionLanguageFeatureLevel expressionLanguageFeatureLevel) {
+		if ( LOG.isDebugEnabled() ) {
+			if ( expressionLanguageFeatureLevel != null ) {
+				LOG.debug( "Setting ExpressionLanguageFeatureLevel for custom violations to " + expressionLanguageFeatureLevel.name() );
+			}
+		}
+		this.customViolationExpressionLanguageFeatureLevel = expressionLanguageFeatureLevel;
+		return thisAsT();
+	}
+
+	public ExpressionLanguageFeatureLevel getCustomViolationExpressionLanguageFeatureLevel() {
+		return customViolationExpressionLanguageFeatureLevel;
+	}
 
 	public final Set<DefaultConstraintMapping> getProgrammaticMappings() {
 		return programmaticMappings;

engine/src/main/java/org/hibernate/validator/internal/engine/MessageInterpolatorContext.java

@@ -17,6 +17,7 @@
 import org.hibernate.validator.internal.util.logging.Log;
 import org.hibernate.validator.internal.util.logging.LoggerFactory;
 import org.hibernate.validator.internal.util.stereotypes.Immutable;
+import org.hibernate.validator.messageinterpolation.ExpressionLanguageFeatureLevel;
 import org.hibernate.validator.messageinterpolation.HibernateMessageInterpolatorContext;
 
 /**
@@ -39,22 +40,25 @@ public class MessageInterpolatorContext implements HibernateMessageInterpolatorC
 	private final Map<String, Object> messageParameters;
 	@Immutable
 	private final Map<String, Object> expressionVariables;
-	private final boolean expressionLanguageEnabled;
+	private final ExpressionLanguageFeatureLevel expressionLanguageFeatureLevel;
+	private final boolean customViolation;
 
 	public MessageInterpolatorContext(ConstraintDescriptor<?> constraintDescriptor,
 					Object validatedValue,
 					Class<?> rootBeanType,
 					Path propertyPath,
 					Map<String, Object> messageParameters,
 					Map<String, Object> expressionVariables,
-					boolean expressionLanguageEnabled) {
+					ExpressionLanguageFeatureLevel expressionLanguageFeatureLevel,
+					boolean customViolation) {
 		this.constraintDescriptor = constraintDescriptor;
 		this.validatedValue = validatedValue;
 		this.rootBeanType = rootBeanType;
 		this.propertyPath = propertyPath;
 		this.messageParameters = toImmutableMap( messageParameters );
 		this.expressionVariables = toImmutableMap( expressionVariables );
-		this.expressionLanguageEnabled = expressionLanguageEnabled;
+		this.expressionLanguageFeatureLevel = expressionLanguageFeatureLevel;
+		this.customViolation = customViolation;
 	}
 
 	@Override
@@ -78,8 +82,12 @@ public Map<String, Object> getMessageParameters() {
 	}
 
 	@Override
-	public boolean isExpressionLanguageEnabled() {
-		return expressionLanguageEnabled;
+	public ExpressionLanguageFeatureLevel getExpressionLanguageFeatureLevel() {
+		return expressionLanguageFeatureLevel;
+	}
+
+	public boolean isCustomViolation() {
+		return customViolation;
 	}
 
 	@Override
@@ -143,7 +151,8 @@ public String toString() {
 		sb.append( ", propertyPath=" ).append( propertyPath );
 		sb.append( ", messageParameters=" ).append( messageParameters );
 		sb.append( ", expressionVariables=" ).append( expressionVariables );
-		sb.append( ", expressionLanguageEnabled=" ).append( expressionLanguageEnabled );
+		sb.append( ", expressionLanguageFeatureLevel=" ).append( expressionLanguageFeatureLevel );
+		sb.append( ", customViolation=" ).append( customViolation );
 		sb.append( '}' );
 		return sb.toString();
 	}