Fix predictable marker collision in EncodingPatternPreservation

src/main/java/org/owasp/esapi/codecs/ref/EncodingPatternPreservation.java

@@ -2,6 +2,7 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.UUID;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -12,12 +13,35 @@
  *
  */
 public class EncodingPatternPreservation {
-    /** Default replacement marker. */
-    private static final String REPLACEMENT_MARKER = EncodingPatternPreservation.class.getSimpleName();
     /** Pattern that is used to identify which content should be replaced. */
     private final Pattern noEncodeContent;
-    /** The Marker used to replace found Pattern references. */
-    private String replacementMarker = REPLACEMENT_MARKER;
+    /**
+     * The Marker used to replace found Pattern references. Defaults to a
+     * per-instance random value so that input content cannot collide with it;
+     * see {@link #captureAndReplaceMatches(String)}.
+     */
+    private String replacementMarker = defaultReplacementMarker();
+
+    /**
+     * Builds an unpredictable default marker. Using a fixed, publicly-known
+     * marker (e.g. this class's simple name) would let an attacker embed that
+     * literal string in the input ahead of real matched content; since
+     * {@link #restoreOriginalContent(String)} replaces markers in encounter
+     * order via {@code replaceFirst}, that attacker-supplied marker would be
+     * replaced first, desynchronizing every subsequent restoration.
+     * <p>
+     * The marker is kept purely alphanumeric (no hyphens) because callers
+     * such as {@link org.owasp.esapi.codecs.CSSCodec} run an encoding pass
+     * over the marker before it is restored; a hyphen would itself be
+     * escaped by that pass, breaking the exact-match lookup in
+     * {@link #restoreOriginalContent(String)}.
+     *
+     * @return A marker string that is not derivable from public information.
+     */
+    private static String defaultReplacementMarker() {
+        return EncodingPatternPreservation.class.getSimpleName()
+                + UUID.randomUUID().toString().replace("-", "");
+    }
 
     /**
      * The ordered-list of elements that were replaced in the last call to

src/test/java/org/owasp/esapi/codecs/ref/EncodingPatternPreservationTest.java

@@ -15,7 +15,9 @@ public void testReplaceAndRestore() {
         String origStr = "12 ABC 34 DEF 56 G 7";
         String replacedStr = epp.captureAndReplaceMatches(origStr);
 
-        assertEquals("12 EncodingPatternPreservation 34 DEF 56 G 7", replacedStr);
+        assertFalse("Replaced string should no longer contain the matched pattern", replacedStr.contains("ABC"));
+        assertTrue(replacedStr.startsWith("12 EncodingPatternPreservation"));
+        assertTrue(replacedStr.endsWith(" 34 DEF 56 G 7"));
 
         String restored = epp.restoreOriginalContent(replacedStr);
         assertEquals(origStr, restored);
@@ -28,12 +30,41 @@ public void testReplaceMultipleAndRestore() {
         String origStr = "12 ABC 34 ABC 56 G 7 ABC8";
         String replacedStr = epp.captureAndReplaceMatches(origStr);
 
-        assertEquals("12 EncodingPatternPreservation 34 EncodingPatternPreservation 56 G 7 EncodingPatternPreservation8", replacedStr);
+        assertFalse("Replaced string should no longer contain the matched pattern", replacedStr.contains("ABC"));
 
         String restored = epp.restoreOriginalContent(replacedStr);
         assertEquals(origStr, restored);
     }
 
+    @Test
+    public void testDefaultMarkerIsUnpredictablePerInstance() {
+        Pattern numberRegex = Pattern.compile("(ABC)");
+        EncodingPatternPreservation epp1 = new EncodingPatternPreservation(numberRegex);
+        EncodingPatternPreservation epp2 = new EncodingPatternPreservation(numberRegex);
+
+        String replaced1 = epp1.captureAndReplaceMatches("ABC");
+        String replaced2 = epp2.captureAndReplaceMatches("ABC");
+
+        assertNotEquals("Default marker must not be a fixed, guessable value shared across instances",
+                replaced1, replaced2);
+    }
+
+    @Test
+    public void testInputContainingLiteralClassNameDoesNotDesyncRestoration() {
+        // Regression test: previously the default marker was the fixed literal
+        // "EncodingPatternPreservation", so input already containing that literal
+        // string ahead of real matched content would be restored in place of the
+        // real match, desynchronizing every subsequent restoreFirst() call.
+        Pattern numberRegex = Pattern.compile("(ABC)");
+        EncodingPatternPreservation epp = new EncodingPatternPreservation(numberRegex);
+        String origStr = "EncodingPatternPreservation prefix ABC suffix";
+
+        String replacedStr = epp.captureAndReplaceMatches(origStr);
+        String restored = epp.restoreOriginalContent(replacedStr);
+
+        assertEquals(origStr, restored);
+    }
+
     @Test
     public void testSetMarker() {
         Pattern numberRegex = Pattern.compile("(ABC)");