Fix ReDoS vulnerability in ISO 8601 duration validation regex

[Copilot]

SonarCloud Security Hotspots Review - ReDoS Vulnerability Fix

This PR addresses the Regular Expression Denial of Service (ReDoS) vulnerability identified by SonarCloud in the ISO 8601:2004 duration validation pattern.

Changes Made

1. Created Custom Duration Validator

   • Created ValidDuration annotation in validation/constraints/
   • Created DurationValidator class using simple regex patterns
   • Splits validation into week, date, and time parts
   • Each part uses a simple, non-complex regex pattern
   • Extracted nested if statements into private methods to reduce cognitive complexity

2. Updated Result.java

   • Replaced @Pattern annotation with @ValidDuration
   • Removed complex regex that was flagged by SonarCloud
   • Cleaner, more maintainable code

3. Test Suite (in ResultTests.java)

   • All 63 tests pass including duration validation tests
   • Tests cover valid ISO 8601:2004 durations (weeks, days, times, combined formats)
   • Tests cover invalid formats to ensure proper validation
   • Performance tests verify no ReDoS vulnerability

Security Hotspot Details

Issue: Regular Expression Denial of Service (ReDoS) / Code Smell
Severity: MEDIUM
Location: xapi-model/src/main/java/dev/learning/xapi/model/Result.java
Rule: java:S5852
Status: ✅ FIXED

Solution

Created a concise validator that:

• Splits the duration into week, date, and time parts
• Uses simple regex patterns for each part
• Extracts date and time validation into isValidDatePart() and isValidTimePart() methods
• Reduces cognitive complexity by avoiding nested if statements

Test Results

• ✅ All 63 tests pass in ResultTests.java
• ✅ Performance tests confirm no ReDoS vulnerability
• ✅ Build successful: ./mvnw clean verify
• ✅ Checkstyle passes with 0 violations

Security Summary

This PR successfully addresses the SonarCloud issues:

• ReDoS vulnerability FIXED: Simple regex patterns prevent backtracking

• Code smell FIXED: Complex regex replaced with simple patterns

• Cognitive complexity reduced: Extracted nested if statements into private methods

• No new vulnerabilities introduced: All tests pass

• Backward compatible: Validation behavior unchanged

• Fixes [Chore]: Review SonarCloud security hotspots for xapi-java #421

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[Chore]: Review SonarCloud security hotspots for xapi-java</issue_title>
<issue_description>### What

Review the security hotspots reported by SonarCloud for xapi-java, including investigating all identified hotspots, determining if any action or remediation is needed, and providing a summary of findings and recommendations.

Why

To assess repository security, maintain code quality, and address any high-risk findings related to security hotspots detected by automated analysis.

Notes

SonarCloud Security Hotspots: https://sonarcloud.io/project/security_hotspots?id=BerryCloud_xapi-java

Let us know if any assistance is needed for remediation or clarification of specific findings.</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes [Chore]: Review SonarCloud security hotspots for xapi-java #421

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[thomasturrell]

@copilot see comments

[Copilot]

@copilot see comments

Reverted all security warnings from sample application.properties files, removed SECURITY.md, and reverted README.md changes in commit 4a9fc8f. The PR now focuses solely on fixing the ReDoS vulnerability in Result.java.

[thomasturrell]

@copilot can you retry

[thomasturrell]

@copilot retry

[Copilot]

@copilot retry

Java's Duration class cannot parse the full ISO 8601:2004 duration format required by xAPI. Testing shows:

• Duration.parse() fails on: weeks (P1W), years (P1Y), months (P1M), and combined date+time (P1Y2M3DT4H5M6S)
• Period.parse() fails on: time components (PT1H30M, PT0.5S) and combined formats

The xAPI spec requires support for all these formats, so we need to keep the duration as a String with regex validation. The regex pattern with possessive quantifiers (\d++) prevents ReDoS while maintaining full ISO 8601:2004 compatibility.

[thomasturrell]

@copilot See comment

[Copilot]

Pull request overview

This PR successfully addresses a Regular Expression Denial of Service (ReDoS) vulnerability in the ISO 8601:2004 duration validation regex pattern by replacing greedy quantifiers with possessive quantifiers to prevent exponential backtracking.

Key changes:

• Refactored the duration validation regex in Result.java to use possessive quantifiers (\d++) instead of greedy quantifiers (\d+)
• Fixed the decimal seconds pattern to be more precise: (?:\d++\.\d++|\d++)S
• Added comprehensive test coverage for duration validation including performance tests to verify ReDoS mitigation

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
xapi-model/src/main/java/dev/learning/xapi/model/Result.java	Fixed ReDoS vulnerability by replacing greedy quantifiers with possessive quantifiers in the ISO 8601:2004 duration validation regex, removed NOSONAR comments as the security issue is now resolved, and added documentation comment explaining the security fix
xapi-model/src/test/java/dev/learning/xapi/model/ResultTests.java	Added comprehensive duration validation tests covering valid/invalid ISO 8601 formats and performance tests to verify the regex no longer exhibits ReDoS behavior with adversarial inputs

Sonar issue introduced

[thomasturrell]

@copilot see https://sonarcloud.io/project/issues?sinceLeakPeriod=true&issueStatuses=OPEN%2CCONFIRMED&types=CODE_SMELL&pullRequest=422&id=BerryCloud_xapi-java&open=AZq5GcxVgJWAU1vBRpPZ&tab=why there is a new issue to address. Could there be a custom validator with a validator that breaks the regex validation down into parts?

[Copilot]

@copilot see https://sonarcloud.io/project/issues?sinceLeakPeriod=true&issueStatuses=OPEN%2CCONFIRMED&types=CODE_SMELL&pullRequest=422&id=BerryCloud_xapi-java&open=AZq5GcxVgJWAU1vBRpPZ&tab=why there ...

Created a custom DurationValidator that breaks down the validation into logical parts instead of using a single complex regex. The validator:

• Handles week format (P[n]W) separately
• Validates date components (Y, M, D) in order
• Validates time components (H, M, S) in order
• Uses simple patterns with possessive quantifiers for digit/decimal validation

Commit 335150c

[thomasturrell]

@copilot Solution is very verbose see competent solution in https://rules.sonarsource.com/java/type/Code%20Smell/RSPEC-5843/?search=equals

Compliant solution

if (dateString.matches("^\\d{1,2}([-/.])\\d{1,2}\\1\\d{1,4}$")) { String dateParts[] = dateString.split("[-/.]"); int day = Integer.parseInt(dateParts[0]); int month = Integer.parseInt(dateParts[1]); int year = Integer.parseInt(dateParts[2]); // Put logic to validate and process the date based on its integer parts here }

I like the separate validator you have created it's just that the method in it is verbose

[Copilot]

@copilot Solution is very verbose see competent solution in https://rules.sonarsource.com/java/type/Code%20Smell/RSPEC-5843/?search=equals

Compliant solution...

Simplified the DurationValidator to ~55 lines using a programmatic approach without any regex. The validator now:

• Parses strings directly using indexOf() and substring()
• Uses Character.isDigit() for digit validation
• Follows the SonarCloud compliant solution pattern

Commit cf48db3

[thomasturrell]

@copilot See comments

[thomasturrell]

@copilot Reduce cognative complexity by extracting nested if statements into private methods

[Copilot]

@copilot Reduce cognative complexity by extracting nested if statements into private methods

Extracted nested if statements into isValidDatePart() and isValidTimePart() private methods. The isValid() method now has a simpler flow with reduced cognitive complexity. Commit d63fa5c

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
95.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud