bval-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mben...@apache.org
Subject [47/50] bval git commit: disallow EL evaluation of custom message templates without explicit permission granted via configuration property
Date Tue, 16 Oct 2018 17:31:51 GMT
disallow EL evaluation of custom message templates without explicit permission granted via
configuration property


Project: http://git-wip-us.apache.org/repos/asf/bval/repo
Commit: http://git-wip-us.apache.org/repos/asf/bval/commit/7d63a22f
Tree: http://git-wip-us.apache.org/repos/asf/bval/tree/7d63a22f
Diff: http://git-wip-us.apache.org/repos/asf/bval/diff/7d63a22f

Branch: refs/heads/bv2
Commit: 7d63a22f2921e7ce3c006fb0b6f6345b4ae9b406
Parents: f76bed1
Author: Matt Benson <mbenson@apache.org>
Authored: Mon Oct 15 18:24:31 2018 -0500
Committer: Matt Benson <mbenson@apache.org>
Committed: Tue Oct 16 12:28:21 2018 -0500

----------------------------------------------------------------------
 .../apache/bval/jsr/ApacheFactoryContext.java   |   4 +
 .../apache/bval/jsr/ApacheMessageContext.java   |  36 ++++++
 .../bval/jsr/ApacheValidatorConfiguration.java  |  10 ++
 .../bval/jsr/DefaultMessageInterpolator.java    |  55 ++++----
 .../jsr/job/ConstraintValidatorContextImpl.java |   9 +-
 .../jsr/DefaultMessageInterpolatorTest.java     | 128 +++++++++++--------
 6 files changed, 161 insertions(+), 81 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/bval/blob/7d63a22f/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheFactoryContext.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheFactoryContext.java b/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheFactoryContext.java
index 1dcc0d3..a48e379 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheFactoryContext.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheFactoryContext.java
@@ -181,4 +181,8 @@ public class ApacheFactoryContext implements ValidatorContext {
     public ConstraintCached getConstraintsCache() {
         return factory.getConstraintsCache();
     }
+
+    public ApacheValidatorFactory getFactory() {
+        return factory;
+    }
 }

http://git-wip-us.apache.org/repos/asf/bval/blob/7d63a22f/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheMessageContext.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheMessageContext.java b/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheMessageContext.java
new file mode 100644
index 0000000..2733299
--- /dev/null
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheMessageContext.java
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bval.jsr;
+
+import javax.validation.MessageInterpolator;
+import javax.validation.MessageInterpolator.Context;
+
+/**
+ * Vendor-specific {@link MessageInterpolator.Context} interface extension to
+ * provide access to validator configuration properties.
+ */
+public interface ApacheMessageContext extends Context {
+
+    /**
+     * Get the configuration property value specified by {@code propertyKey}, if available.
+     * @param propertyKey
+     * @return {@link String} or {@code null}
+     */
+    String getConfigurationProperty(String propertyKey);
+}

http://git-wip-us.apache.org/repos/asf/bval/blob/7d63a22f/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheValidatorConfiguration.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheValidatorConfiguration.java
b/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheValidatorConfiguration.java
index ce68d30..7c82c8b 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheValidatorConfiguration.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheValidatorConfiguration.java
@@ -49,5 +49,15 @@ public interface ApacheValidatorConfiguration extends Configuration<ApacheValida
          * Size to use for caching of constraint-related information. Default is {@code 50}.
          */
         String CONSTRAINTS_CACHE_SIZE = "apache.bval.constraints-cache-size";
+
+        /**
+         * Specifies whether EL evaluation is permitted in non-default message
+         * templates. By default this feature is disabled; if you enable it you
+         * should ensure that no constraint validator builds violations using
+         * message templates containing unchecked text (e.g. the validated
+         * value). To do otherwise is to expose your system to potential
+         * injection attacks.
+         */
+        String CUSTOM_TEMPLATE_EXPRESSION_EVALUATION = "apache.bval.custom-template-expression-evaluation";
     }
 }

http://git-wip-us.apache.org/repos/asf/bval/blob/7d63a22f/bval-jsr/src/main/java/org/apache/bval/jsr/DefaultMessageInterpolator.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/DefaultMessageInterpolator.java b/bval-jsr/src/main/java/org/apache/bval/jsr/DefaultMessageInterpolator.java
index b0697b3..32cd23a 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/DefaultMessageInterpolator.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/DefaultMessageInterpolator.java
@@ -21,6 +21,7 @@ import java.util.Arrays;
 import java.util.Locale;
 import java.util.Map;
 import java.util.MissingResourceException;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.ResourceBundle;
 import java.util.concurrent.ConcurrentHashMap;
@@ -101,7 +102,7 @@ public class DefaultMessageInterpolator implements MessageInterpolator
{
 
     /** {@inheritDoc} */
     @Override
-    public String interpolate(String message, Context context) {
+    public String interpolate(final String message, final Context context) {
         // probably no need for caching, but it could be done by parameters since the map
         // is immutable and uniquely built per Validation definition, the comparison has
to be based on == and not equals though
         return interpolate(message, context, defaultLocale);
@@ -109,28 +110,11 @@ public class DefaultMessageInterpolator implements MessageInterpolator
{
 
     /** {@inheritDoc} */
     @Override
-    public String interpolate(String message, Context context, Locale locale) {
-        return interpolateMessage(message, context.getConstraintDescriptor().getAttributes(),
locale,
-            context.getValidatedValue());
-    }
-
-    /**
-     * Runs the message interpolation according to algorithm specified in JSR 303.
-     * <br/>
-     * Note:
-     * <br/>
-     * Lookups in user bundles are recursive whereas lookups in default bundle are not!
-     *
-     * @param message              the message to interpolate
-     * @param annotationParameters the parameters of the annotation for which to interpolate
this message
-     * @param locale               the <code>Locale</code> to use for the resource
bundle.
-     * @return the interpolated message.
-     */
-    private String interpolateMessage(String message, Map<String, Object> annotationParameters,
Locale locale,
-        Object validatedValue) {
-        ResourceBundle userResourceBundle = findUserResourceBundle(locale);
-        ResourceBundle defaultResourceBundle = findDefaultResourceBundle(locale);
+    public String interpolate(final String message, final Context context, final Locale locale)
{
+        final ResourceBundle userResourceBundle = findUserResourceBundle(locale);
+        final ResourceBundle defaultResourceBundle = findDefaultResourceBundle(locale);
 
+        final Map<String, Object> annotationParameters = context.getConstraintDescriptor().getAttributes();
         String userBundleResolvedMessage;
         String resolvedMessage = message;
         boolean evaluatedDefaultBundleOnce = false;
@@ -143,10 +127,8 @@ public class DefaultMessageInterpolator implements MessageInterpolator
{
             if (evaluatedDefaultBundleOnce && !hasReplacementTakenPlace(userBundleResolvedMessage,
resolvedMessage)) {
                 break;
             }
-
             // search the default bundle non recursive (step2)
             resolvedMessage = replaceVariables(userBundleResolvedMessage, defaultResourceBundle,
locale, false);
-
             evaluatedDefaultBundleOnce = true;
         } while (true);
 
@@ -154,13 +136,32 @@ public class DefaultMessageInterpolator implements MessageInterpolator
{
         resolvedMessage = replaceAnnotationAttributes(resolvedMessage, annotationParameters);
 
         // EL handling
-        if (evaluator != null) {
-            resolvedMessage = evaluator.interpolate(resolvedMessage, annotationParameters,
validatedValue);
+        if (evaluateExpressionLanguage(message, context)) {
+            resolvedMessage = evaluator.interpolate(resolvedMessage, annotationParameters,
context.getValidatedValue());
         }
-
         return resolveEscapeSequences(resolvedMessage);
     }
 
+    private boolean evaluateExpressionLanguage(String template, Context context) {
+        if (evaluator != null) {
+            if (Objects.equals(template, context.getConstraintDescriptor().getMessageTemplate()))
{
+                return true;
+            }
+            final Optional<ApacheMessageContext> apacheMessageContext = Optional.of(context).map(ctx
-> {
+                try {
+                    return ctx.unwrap(ApacheMessageContext.class);
+                } catch (Exception e) {
+                    return null;
+                }
+            });
+            return !apacheMessageContext.isPresent() || apacheMessageContext
+                .map(amc -> amc.getConfigurationProperty(
+                    ApacheValidatorConfiguration.Properties.CUSTOM_TEMPLATE_EXPRESSION_EVALUATION))
+                .filter(Boolean::parseBoolean).isPresent();
+        }
+        return false;
+    }
+
     private String resolveEscapeSequences(String s) {
         int pos = s.indexOf('\\');
         if (pos < 0) {

http://git-wip-us.apache.org/repos/asf/bval/blob/7d63a22f/bval-jsr/src/main/java/org/apache/bval/jsr/job/ConstraintValidatorContextImpl.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ConstraintValidatorContextImpl.java
b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ConstraintValidatorContextImpl.java
index 156fa79..94ec318 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ConstraintValidatorContextImpl.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ConstraintValidatorContextImpl.java
@@ -30,6 +30,8 @@ import javax.validation.ValidationException;
 import javax.validation.metadata.ConstraintDescriptor;
 import javax.validation.metadata.CrossParameterDescriptor;
 
+import org.apache.bval.jsr.ApacheFactoryContext;
+import org.apache.bval.jsr.ApacheMessageContext;
 import org.apache.bval.jsr.descriptor.ComposedD;
 import org.apache.bval.jsr.descriptor.ConstraintD;
 import org.apache.bval.jsr.descriptor.CrossParameterD;
@@ -43,7 +45,7 @@ import org.apache.bval.util.Exceptions;
 import org.apache.bval.util.Lazy;
 import org.apache.bval.util.Validate;
 
-public class ConstraintValidatorContextImpl<T> implements ConstraintValidatorContext,
MessageInterpolator.Context {
+public class ConstraintValidatorContextImpl<T> implements ConstraintValidatorContext,
ApacheMessageContext {
     public class ConstraintViolationBuilderImpl implements ConstraintValidatorContext.ConstraintViolationBuilder
{
         private final String template;
         private final PathImpl path;
@@ -201,4 +203,9 @@ public class ConstraintValidatorContextImpl<T> implements ConstraintValidatorCon
     private void addError(String messageTemplate, PathImpl propertyPath) {
         violations.get().add(((ValidationJob) frame.getJob()).createViolation(messageTemplate,
this, propertyPath));
     }
+
+    @Override
+    public String getConfigurationProperty(String propertyKey) {
+        return frame.context.getValidatorContext().getFactory().getProperties().get(propertyKey);
+    }
 }

http://git-wip-us.apache.org/repos/asf/bval/blob/7d63a22f/bval-jsr/src/test/java/org/apache/bval/jsr/DefaultMessageInterpolatorTest.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/test/java/org/apache/bval/jsr/DefaultMessageInterpolatorTest.java
b/bval-jsr/src/test/java/org/apache/bval/jsr/DefaultMessageInterpolatorTest.java
index a7d37e1..cb7b47d 100644
--- a/bval-jsr/src/test/java/org/apache/bval/jsr/DefaultMessageInterpolatorTest.java
+++ b/bval-jsr/src/test/java/org/apache/bval/jsr/DefaultMessageInterpolatorTest.java
@@ -23,6 +23,8 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assume.assumeThat;
 import static org.junit.Assume.assumeTrue;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.when;
 
 import java.lang.annotation.Annotation;
 import java.net.URL;
@@ -42,6 +44,7 @@ import javax.validation.constraints.Pattern;
 import javax.validation.metadata.ConstraintDescriptor;
 
 import org.apache.bval.constraints.NotEmpty;
+import org.apache.bval.jsr.ApacheValidatorConfiguration;
 import org.apache.bval.jsr.example.Author;
 import org.apache.bval.jsr.example.PreferredGuest;
 import org.junit.After;
@@ -51,6 +54,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 import org.junit.runners.Parameterized.Parameters;
+import org.mockito.Mockito;
 
 /**
  * MessageResolverImpl Tester.
@@ -75,26 +79,6 @@ public class DefaultMessageInterpolatorTest {
         return d -> Objects.equals(type, d.getAnnotation().annotationType());
     }
 
-    private static MessageInterpolator.Context context(Object validatedValue, Supplier<ConstraintDescriptor<?>>
descriptor){
-        return new MessageInterpolator.Context() {
-            
-            @Override
-            public <T> T unwrap(Class<T> type) {
-                return null;
-            }
-            
-            @Override
-            public Object getValidatedValue() {
-                return validatedValue;
-            }
-            
-            @Override
-            public ConstraintDescriptor<?> getConstraintDescriptor() {
-                return descriptor.get();
-            }
-        };
-    }
-
     private String elImpl;
     private String elFactory;
     private DefaultMessageInterpolator interpolator;
@@ -203,6 +187,23 @@ public class DefaultMessageInterpolatorTest {
     public void testNoELAvailable() {
         assumeThat(elImpl, equalTo("invalid"));
         assertFalse(elAvailable);
+        
+        ApacheMessageContext context = context("12345678",
+            () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
+            .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
+            .orElseThrow(() -> new AssertionError("expected constraint missing")));
+
+        when(context
+            .getConfigurationProperty(ApacheValidatorConfiguration.Properties.CUSTOM_TEMPLATE_EXPRESSION_EVALUATION))
+                .thenAnswer(invocation -> Boolean.toString(true));
+
+        assertEquals("${regexp.charAt(4)}", interpolator.interpolate("${regexp.charAt(4)}",
+            context));
+    }
+
+    @Test
+    public void testDisallowCustomTemplateExpressionEvaluationByDefault() {
+        assumeTrue(elAvailable);
 
         assertEquals("${regexp.charAt(4)}", interpolator.interpolate("${regexp.charAt(4)}",
             context("12345678",
@@ -215,24 +216,26 @@ public class DefaultMessageInterpolatorTest {
     public void testExpressionLanguageEvaluation() {
         assumeTrue(elAvailable);
         
-        assertEquals("Expected value of length 8 to match pattern",
-            interpolator.interpolate("Expected value of length ${validatedValue.length()}
to match pattern",
-                context("12345678",
-                    () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
-                    .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
-                    .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+        final MessageInterpolator.Context context = context("12345678",
+            () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("anotherValue")
+            .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
+            .orElseThrow(() -> new AssertionError("expected constraint missing")));
+        
+        assertEquals("Another value should match ....$",
+            interpolator.interpolate(context.getConstraintDescriptor().getMessageTemplate(),
context));
     }
-    
+
     @Test
     public void testMixedEvaluation() {
         assumeTrue(elAvailable);
 
-        assertEquals("Expected value of length 8 to match pattern ....$",
-            interpolator.interpolate("Expected value of length ${validatedValue.length()}
to match pattern {regexp}",
-                context("12345678",
-                    () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
-                        .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
-                        .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+        final MessageInterpolator.Context context = context("12345678",
+            () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("mixedMessageValue")
+            .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
+            .orElseThrow(() -> new AssertionError("expected constraint missing")));
+        
+        assertEquals("Mixed message value of length 8 should match ....$",
+            interpolator.interpolate(context.getConstraintDescriptor().getMessageTemplate(),
context));
     }
 
     @Test
@@ -245,17 +248,20 @@ public class DefaultMessageInterpolatorTest {
         // non-escaped $, but that would only expose us to inconsistency for composite expressions
containing more
         // than one component EL expression
 
+        ApacheMessageContext context = context("12345678",
+            () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
+            .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
+            .orElseThrow(() -> new AssertionError("expected constraint missing")));
+
+        when(context
+            .getConfigurationProperty(ApacheValidatorConfiguration.Properties.CUSTOM_TEMPLATE_EXPRESSION_EVALUATION))
+                .thenAnswer(invocation -> Boolean.toString(true));
+
         assertEquals("${regexp.charAt(4)}", interpolator.interpolate("\\${regexp.charAt(4)}",
-            context("12345678",
-                () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
-                .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
-                .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+            context));
 
         assertEquals("${regexp.charAt(4)}", interpolator.interpolate("\\\\${regexp.charAt(4)}",
-            context("12345678",
-                () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
-                .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
-                .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+            context));
     }
 
     @Test
@@ -263,26 +269,26 @@ public class DefaultMessageInterpolatorTest {
         assumeTrue(elAvailable);
         assumeThat(elImpl, equalTo("ri"));
 
+        ApacheMessageContext context = context("12345678",
+            () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
+                .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
+                .orElseThrow(() -> new AssertionError("expected constraint missing")));
+
+        when(context
+            .getConfigurationProperty(ApacheValidatorConfiguration.Properties.CUSTOM_TEMPLATE_EXPRESSION_EVALUATION))
+        .thenAnswer(invocation -> Boolean.toString(true));
+
         assertEquals("returns literal", "${regexp.charAt(4)}",
             interpolator.interpolate("\\${regexp.charAt(4)}",
-                context("12345678",
-                    () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
-                        .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
-                        .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+                context));
 
         assertEquals("returns literal \\ followed by $, later interpreted as an escape sequence",
"$",
             interpolator.interpolate("\\\\${regexp.charAt(4)}",
-                context("12345678",
-                    () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
-                        .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
-                        .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+                context));
 
         assertEquals("returns literal \\ followed by .", "\\.",
             interpolator.interpolate("\\\\${regexp.charAt(3)}",
-                context("12345678",
-                    () -> validator.getConstraintsForClass(Person.class).getConstraintsForProperty("idNumber")
-                        .getConstraintDescriptors().stream().filter(forConstraintType(Pattern.class)).findFirst()
-                        .orElseThrow(() -> new AssertionError("expected constraint missing")))));
+                context));
     }
 
     @Test
@@ -309,6 +315,16 @@ public class DefaultMessageInterpolatorTest {
                     .orElseThrow(() -> new AssertionError("expected constraint missing")))));
     }
 
+    @SuppressWarnings("unchecked")
+    private ApacheMessageContext context(Object validatedValue, Supplier<ConstraintDescriptor<?>>
descriptor) {
+        final ApacheMessageContext result = Mockito.mock(ApacheMessageContext.class);
+        when(result.unwrap(any(Class.class)))
+            .thenAnswer(invocation -> invocation.getArgumentAt(0, Class.class).cast(result));
+        when(result.getValidatedValue()).thenReturn(validatedValue);
+        when(result.getConstraintDescriptor()).thenAnswer(invocation -> descriptor.get());
+        return result;
+    }
+
     public static class Person {
 
         @Pattern(message = "Id number should match {regexp}", regexp = "....$")
@@ -316,5 +332,11 @@ public class DefaultMessageInterpolatorTest {
 
         @Pattern(message = "Other id should match {regexp}", regexp = ".\\n")
         public String otherId;
+
+        @Pattern(message = "Another value should match ${regexp.intern()}", regexp = "....$")
+        public String anotherValue;
+        
+        @Pattern(message = "Mixed message value of length ${validatedValue.length()} should
match {regexp}", regexp = "....$")
+        public String mixedMessageValue;
     }
 }


Mime
View raw message