Fix ReDoS vulnerability in ISO 8601 duration validation regex (#422)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: thomasturrell <1552612+thomasturrell@users.noreply.github.com>

Author: Copilot

xapi-model/src/main/java/dev/learning/xapi/model/Result.java

@@ -8,8 +8,8 @@
 import com.fasterxml.jackson.annotation.JsonInclude.Include;
 import dev.learning.xapi.model.validation.constraints.HasScheme;
 import dev.learning.xapi.model.validation.constraints.VaildScore;
+import dev.learning.xapi.model.validation.constraints.ValidDuration;
 import jakarta.validation.Valid;
-import jakarta.validation.constraints.Pattern;
 import java.net.URI;
 import java.util.LinkedHashMap;
 import java.util.function.Consumer;
@@ -41,14 +41,7 @@ public class Result {
   private String response;
 
   /** Period of time over which the Statement occurred. */
-  // Java Duration does not store ISO 8601:2004 durations.
-  @Pattern(
-      regexp =
-          "^(P\\d+W)?$|^P(?!$)(\\d+Y)?(\\d+M)?" // NOSONAR
-              + "(\\d+D)?(T(?=\\d)(\\d+H)?(\\d+M)?(\\d*\\.?\\d+S)?)?$", // NOSONAR
-      flags = Pattern.Flag.CASE_INSENSITIVE,
-      message = "Must be a valid ISO 8601:2004 duration format.")
-  private String duration;
+  @ValidDuration private String duration;
 
   private LinkedHashMap<@HasScheme URI, Object> extensions;
 

xapi-model/src/main/java/dev/learning/xapi/model/validation/constraints/ValidDuration.java

@@ -0,0 +1,64 @@
+/*
+ * Copyright 2016-2025 Berry Cloud Ltd. All rights reserved.
+ */
+
+package dev.learning.xapi.model.validation.constraints;
+
+import static java.lang.annotation.ElementType.ANNOTATION_TYPE;
+import static java.lang.annotation.ElementType.CONSTRUCTOR;
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.PARAMETER;
+import static java.lang.annotation.ElementType.TYPE_USE;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import dev.learning.xapi.model.validation.internal.validators.DurationValidator;
+import jakarta.validation.Constraint;
+import jakarta.validation.Payload;
+import java.lang.annotation.Documented;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+/**
+ * The annotated element must be a valid ISO 8601:2004 duration format.
+ *
+ * <p>Accepts formats like:
+ *
+ * <ul>
+ *   <li>Week format: P1W, P52W
+ *   <li>Day format: P1D, P365D
+ *   <li>Time format: PT1H, PT30M, PT45S, PT1.5S
+ *   <li>Combined format: P1Y2M3D, P1DT1H30M45S
+ * </ul>
+ *
+ * @author Berry Cloud
+ * @see <a href="https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Data.md#result">xAPI
+ *     Result</a>
+ */
+@Documented
+@Constraint(validatedBy = {DurationValidator.class})
+@Target({METHOD, FIELD, ANNOTATION_TYPE, CONSTRUCTOR, PARAMETER, TYPE_USE})
+@Retention(RUNTIME)
+public @interface ValidDuration {
+
+  /**
+   * Error Message.
+   *
+   * @return the error message
+   */
+  String message() default "Must be a valid ISO 8601:2004 duration format.";
+
+  /**
+   * Groups.
+   *
+   * @return the validation groups
+   */
+  Class<?>[] groups() default {};
+
+  /**
+   * Payload.
+   *
+   * @return the payload
+   */
+  Class<? extends Payload>[] payload() default {};
+}

xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java

@@ -0,0 +1,71 @@
+/*
+ * Copyright 2016-2025 Berry Cloud Ltd. All rights reserved.
+ */
+
+package dev.learning.xapi.model.validation.internal.validators;
+
+import dev.learning.xapi.model.validation.constraints.ValidDuration;
+import jakarta.validation.ConstraintValidator;
+import jakarta.validation.ConstraintValidatorContext;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Validates ISO 8601:2004 duration format strings.
+ *
+ * <p>Supports formats: P[n]W, P[n]Y[n]M[n]DT[n]H[n]M[n]S and variations.
+ *
+ * @author Berry Cloud
+ */
+public class DurationValidator implements ConstraintValidator<ValidDuration, String> {
+
+  // Simple patterns - each validates a single component type
+  private static final Pattern WEEK = Pattern.compile("^\\d+W$", Pattern.CASE_INSENSITIVE);
+  private static final Pattern DATE =
+      Pattern.compile("^(\\d+Y)?(\\d+M)?(\\d+D)?$", Pattern.CASE_INSENSITIVE);
+  private static final Pattern TIME =
+      Pattern.compile("^(\\d+H)?(\\d+M)?((\\d+\\.\\d+|\\d+)S)?$", Pattern.CASE_INSENSITIVE);
+
+  @Override
+  public boolean isValid(String value, ConstraintValidatorContext context) {
+    if (value == null) {
+      return true;
+    }
+
+    if (!value.toUpperCase().startsWith("P") || value.length() < 2) {
+      return false;
+    }
+
+    String rest = value.substring(1);
+
+    if (WEEK.matcher(rest).matches()) {
+      return true;
+    }
+
+    int tpos = rest.toUpperCase().indexOf('T');
+    String datePart = tpos >= 0 ? rest.substring(0, tpos) : rest;
+    String timePart = tpos >= 0 ? rest.substring(tpos + 1) : "";
+
+    if (datePart.isEmpty() && timePart.isEmpty()) {
+      return false;
+    }
+
+    return isValidDatePart(datePart) && isValidTimePart(timePart);
+  }
+
+  private boolean isValidDatePart(String datePart) {
+    if (datePart.isEmpty()) {
+      return true;
+    }
+    Matcher m = DATE.matcher(datePart);
+    return m.matches() && (m.group(1) != null || m.group(2) != null || m.group(3) != null);
+  }
+
+  private boolean isValidTimePart(String timePart) {
+    if (timePart.isEmpty()) {
+      return true;
+    }
+    Matcher m = TIME.matcher(timePart);
+    return m.matches() && (m.group(1) != null || m.group(2) != null || m.group(3) != null);
+  }
+}

xapi-model/src/test/java/dev/learning/xapi/model/ResultTests.java

@@ -5,13 +5,22 @@
 package dev.learning.xapi.model;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import jakarta.validation.ConstraintViolation;
+import jakarta.validation.Validation;
+import jakarta.validation.Validator;
 import java.io.IOException;
+import java.util.Set;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.springframework.util.ResourceUtils;
 
 /**
@@ -25,6 +34,8 @@ class ResultTests {
 
   private final ObjectMapper objectMapper = new ObjectMapper().findAndRegisterModules();
 
+  private final Validator validator = Validation.buildDefaultValidatorFactory().getValidator();
+
   @Test
   void whenDeserializingResultThenResultIsInstanceOfResult() throws Exception {
 
@@ -139,4 +150,184 @@ void whenCallingToStringThenResultIsExpected() {
             "Result(score=Score(scaled=1.0, raw=1.0, min=0.0, max=5.0), success=true, completion=true, "
                 + "response=test, duration=P1D, extensions=null)"));
   }
+
+  // Duration Validation Tests
+
+  @ParameterizedTest
+  @ValueSource(
+      strings = {
+        // Week format
+        "P1W",
+        "P52W",
+        "P104W",
+        // Day format
+        "P1D",
+        "P365D",
+        // Time format
+        "PT1H",
+        "PT30M",
+        "PT45S",
+        "PT1.5S",
+        "PT0.5S",
+        // Combined date format
+        "P1Y",
+        "P1M",
+        "P1Y2M",
+        "P1Y2M3D",
+        // Combined date and time format
+        "P1DT1H",
+        "P1DT1H30M",
+        "P1DT1H30M45S",
+        "P1Y2M3DT4H5M6S",
+        "P1Y2M3DT4H5M6.7S",
+        // Minimal valid formats
+        "PT0S",
+        "PT1S",
+        "P0D",
+        // Real-world examples
+        "PT8H",
+        "P90D",
+        "P2Y",
+        "PT15M30S"
+      })
+  @DisplayName("When Duration Is Valid Then Validation Passes")
+  void whenDurationIsValidThenValidationPasses(String duration) {
+
+    // Given Result With Valid Duration
+    final var result = Result.builder().duration(duration).build();
+
+    // When Validating Result
+    final Set<ConstraintViolation<Result>> violations = validator.validate(result);
+
+    // Then Validation Passes
+    assertThat(violations, empty());
+  }
+
+  @ParameterizedTest
+  @ValueSource(
+      strings = {
+        // Invalid formats
+        "",
+        "T1H",
+        "1D",
+        "PD",
+        "PT",
+        "P1",
+        "1Y2M",
+        // Invalid time without T
+        "P1H",
+        "P1M30S",
+        // Invalid mixing weeks with other units
+        "P1W1D",
+        "P1W1Y",
+        "P1WT1H",
+        // Invalid decimal placement
+        "P1.5D",
+        "P1.5Y",
+        "PT1.5H",
+        "PT1.5M",
+        // Missing P prefix
+        "1Y2M3D",
+        "T1H30M",
+        // Invalid order
+        "P1D1Y",
+        "PT1S1M",
+        "PT1M1H",
+        // Double separators
+        "P1Y2M3DTT1H",
+        "PP1D",
+        // Negative values
+        "P-1D",
+        "PT-1H"
+      })
+  @DisplayName("When Duration Is Invalid Then Validation Fails")
+  void whenDurationIsInvalidThenValidationFails(String duration) {
+
+    // Given Result With Invalid Duration
+    final var result = Result.builder().duration(duration).build();
+
+    // When Validating Result
+    final Set<ConstraintViolation<Result>> violations = validator.validate(result);
+
+    // Then Validation Fails
+    assertThat(violations, not(empty()));
+    assertThat(violations, hasSize(1));
+  }
+
+  @Test
+  @DisplayName("When Duration Is Null Then Validation Passes")
+  void whenDurationIsNullThenValidationPasses() {
+
+    // Given Result With Null Duration
+    final var result = Result.builder().duration(null).build();
+
+    // When Validating Result
+    final Set<ConstraintViolation<Result>> violations = validator.validate(result);
+
+    // Then Validation Passes
+    assertThat(violations, empty());
+  }
+
+  @Test
+  @DisplayName("When Duration Has Many Digits Then Validation Completes Quickly")
+  void whenDurationHasManyDigitsThenValidationCompletesQuickly() {
+
+    // Given Result With Long But Valid Duration
+    final var result = Result.builder().duration("P99999Y99999M99999DT99999H99999M99999S").build();
+
+    // When Validating Result
+    final long startTime = System.nanoTime();
+    final Set<ConstraintViolation<Result>> violations = validator.validate(result);
+    final long endTime = System.nanoTime();
+    final long durationMs = (endTime - startTime) / 1_000_000;
+
+    // Then Validation Passes And Completes In Reasonable Time
+    assertThat(violations, empty());
+    // Validation should complete quickly - 500ms is generous for a regex match
+    assertThat("Validation should complete in less than 500ms", durationMs < 500);
+  }
+
+  @Test
+  @DisplayName("When Duration Is Adversarial Input Then Validation Completes Quickly Without ReDoS")
+  void whenDurationIsAdversarialInputThenValidationCompletesQuicklyWithoutReDoS() {
+
+    // Given Result With Adversarial Input That Could Cause ReDoS
+    // This input is designed to trigger exponential backtracking in vulnerable regex patterns
+    final var adversarialInput = "P" + "9".repeat(50) + "!";
+    final var result = Result.builder().duration(adversarialInput).build();
+
+    // When Validating Result
+    final long startTime = System.nanoTime();
+    final Set<ConstraintViolation<Result>> violations = validator.validate(result);
+    final long endTime = System.nanoTime();
+    final long durationMs = (endTime - startTime) / 1_000_000;
+
+    // Then Validation Fails Quickly (not vulnerable to ReDoS)
+    assertThat(violations, not(empty()));
+    // Validation should complete quickly even with adversarial input - 500ms is generous
+    assertThat(
+        "Validation should complete in less than 500ms even with adversarial input",
+        durationMs < 500);
+  }
+
+  @Test
+  @DisplayName("When Duration Has Multiple Optional Groups Then Validation Completes Quickly")
+  void whenDurationHasMultipleOptionalGroupsThenValidationCompletesQuickly() {
+
+    // Given Result With Input That Tests Multiple Optional Groups
+    // This tests the pattern with input that doesn't match but exercises optional groups
+    final var testInput = "PYMDTHMS";
+    final var result = Result.builder().duration(testInput).build();
+
+    // When Validating Result
+    final long startTime = System.nanoTime();
+    final Set<ConstraintViolation<Result>> violations = validator.validate(result);
+    final long endTime = System.nanoTime();
+    final long durationMs = (endTime - startTime) / 1_000_000;
+
+    // Then Validation Fails Quickly Without Excessive Backtracking
+    assertThat(violations, not(empty()));
+    // Validation should complete quickly - 500ms is generous for a regex match
+    assertThat("Validation should complete in less than 500ms", durationMs < 500);
+  }
 }