Resolve RCE vulnerability.

Make tests a bit more resilient to exception message changes. fixes jmrozanec#461
NielsDoucet · Oct 23, 2021 · 9c93c17 · 9c93c17
1 parent 2cf9697
commit 9c93c17
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 9 deletions.
diff --git a/src/main/java/com/cronutils/parser/CronParser.java b/src/main/java/com/cronutils/parser/CronParser.java
@@ -128,7 +128,7 @@ public Cron parse(final String expression) {
                 }
                 return new SingleCron(cronDefinition, results).validate();
             } catch (final IllegalArgumentException e) {
-                throw new IllegalArgumentException(String.format("Failed to parse '%s'. %s", expression, e.getMessage()), e);
+                throw new IllegalArgumentException(String.format("Failed to parse cron expression. %s", e.getMessage()), e);
             }
         }
     }

diff --git a/src/test/java/com/cronutils/Issue418Test.java b/src/test/java/com/cronutils/Issue418Test.java
@@ -5,6 +5,7 @@
 import com.cronutils.model.definition.CronDefinitionBuilder;
 import com.cronutils.model.time.ExecutionTime;
 import com.cronutils.parser.CronParser;
+import org.hamcrest.core.StringEndsWith;
 import org.junit.Test;
 
 import java.time.LocalDate;
@@ -13,8 +14,8 @@
 import java.time.ZonedDateTime;
 import java.util.Optional;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.fail;
+import static org.hamcrest.core.StringEndsWith.endsWith;
+import static org.junit.Assert.*;
 
 public class Issue418Test {
 
@@ -59,7 +60,7 @@ public void testInvalidWeekDayStart() {
             parser.parse("0 0 2 ? * 0/7 *");
             fail("Expected exception for invalid expression");
         } catch (IllegalArgumentException expected) {
-            assertEquals("Failed to parse '0 0 2 ? * 0/7 *'. Value 0 not in range [1, 7]", expected.getMessage());
+            assertThat(expected.getMessage(), endsWith("Value 0 not in range [1, 7]"));
         }
     }
 
@@ -71,7 +72,7 @@ public void testInvalidWeekDayEnd() {
             parser.parse("0 0 2 ? * 1/8 *");
             fail("Expected exception for invalid expression");
         } catch (IllegalArgumentException expected) {
-            assertEquals("Failed to parse '0 0 2 ? * 1/8 *'. Period 8 not in range [1, 7]", expected.getMessage());
+            assertThat(expected.getMessage(), endsWith("Period 8 not in range [1, 7]"));
         }
     }
 }
diff --git a/src/test/java/com/cronutils/parser/CronParserQuartzIntegrationTest.java b/src/test/java/com/cronutils/parser/CronParserQuartzIntegrationTest.java
@@ -20,16 +20,19 @@
 import com.cronutils.model.definition.CronDefinitionBuilder;
 import com.cronutils.model.field.expression.FieldExpressionFactory;
 import com.cronutils.model.time.ExecutionTime;
+import org.hamcrest.core.StringEndsWith;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.internal.matchers.ThrowableMessageMatcher;
 import org.junit.rules.ExpectedException;
 
 import java.time.ZonedDateTime;
 import java.util.Locale;
 import java.util.Optional;
 
 import static org.junit.Assert.*;
+import static org.junit.internal.matchers.ThrowableMessageMatcher.hasMessage;
 
 public class CronParserQuartzIntegrationTest {
 
@@ -248,9 +251,7 @@ public void testReportedErrorContainsSameExpressionAsProvided() {
     public void testMissingExpressionAndInvalidCharsInErrorMessage() {
         thrown.expect(IllegalArgumentException.class);
         final String cronexpression = "* * -1 * * ?";
-        thrown.expectMessage(
-                String.format("Failed to parse '%s'. Invalid expression! Expression: -1 does not describe a range. Negative numbers are not allowed.",
-                        cronexpression));
+        thrown.expect(hasMessage(StringEndsWith.endsWith("Invalid expression! Expression: -1 does not describe a range. Negative numbers are not allowed.")));
         assertNotNull(ExecutionTime.forCron(parser.parse(cronexpression)));
     }
 

diff --git a/src/test/java/com/cronutils/validation/CronValidatorTest.java b/src/test/java/com/cronutils/validation/CronValidatorTest.java
@@ -4,6 +4,8 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.validation.ConstraintViolation;
 import javax.validation.Validation;
@@ -16,6 +18,8 @@
 @RunWith(Parameterized.class)
 public class CronValidatorTest {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger(CronValidatorTest.class);
+
     private final Validator validator = Validation.buildDefaultValidatorFactory().getValidator();
 
     private final String expression;
@@ -38,14 +42,17 @@ public static Object[] expressions() {
                 {"0 0 0 25 12 ?", true},
                 {"0 0 0 L 12 ?", false},
                 {"1,2, * * * * *", false},
-                {"1- * * * * *", false}
+                {"1- * * * * *", false},
+                // Verification for RCE security vulnerability fix: https://github.com/jmrozanec/cron-utils/issues/461
+                {"java.lang.Runtime.getRuntime().exec('touch /tmp/pwned'); // 4 5 [${''.getClass().forName('javax.script.ScriptEngineManager').newInstance().getEngineByName('js').eval(validatedValue)}]", false}
         };
     }
 
     @Test
     public void validateExamples() {
         TestPojo testPojo = new TestPojo(expression);
         Set<ConstraintViolation<TestPojo>> violations = validator.validate(testPojo);
+        violations.stream().map(ConstraintViolation::getMessage).forEach(LOGGER::info);
 
         if (valid) {
             assertTrue(violations.isEmpty());