Avoid excessive backtracking when parsing unterminated strings

3 files changed, 44 insertions, 2 deletions

diff --git a/NEWS b/NEWS
index 5ee0c76..fa7fdb7 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,15 @@
+# 1.12.2
+
+* Avoid excessive backtracking when parsing an unterminated string with many
+  escape sequences. The previous behaviour would allow maliciously crafted
+  documents to cause a denial of service in other implementations of Mammoth.
+  While the exact test case did not cause the same issue in the Java
+  implementation, the behaviour has been updated out of an abundance of caution
+  and for consistency.
+
+  Note that it is still strongly recommended to process untrusted documents in
+  a separate thread with a timeout to avoid potential similar issues.
+
 # 1.12.1
 
 * Detect and ignore numbering levels that use numStyleLink to refer to
diff --git a/src/main/java/org/zwobble/mammoth/internal/styles/parsing/StyleMappingTokeniser.java b/src/main/java/org/zwobble/mammoth/internal/styles/parsing/StyleMappingTokeniser.java
index 9625d24..6afec90 100644
--- a/src/main/java/org/zwobble/mammoth/internal/styles/parsing/StyleMappingTokeniser.java
+++ b/src/main/java/org/zwobble/mammoth/internal/styles/parsing/StyleMappingTokeniser.java
@@ -13,7 +13,7 @@ public class StyleMappingTokeniser {
     }
 
     public static List<Token<TokenType>> tokeniseToList(String line) {
-        String stringPrefix = "'(?:(?:\\\\.|[^'])*)";
+        String stringPrefix = "'(?:(?:\\\\(?:.|$)|[^'\\\\])*)";
         String identifierCharacter = "(?:[a-zA-Z\\-_]|\\\\.)";
 
         RegexTokeniser<TokenType> tokeniser = new RegexTokeniser<>(
diff --git a/src/test/java/org/zwobble/mammoth/tests/styles/parsing/StyleMappingTokeniserTests.java b/src/test/java/org/zwobble/mammoth/tests/styles/parsing/StyleMappingTokeniserTests.java
index f70fd77..18f9397 100644
--- a/src/test/java/org/zwobble/mammoth/tests/styles/parsing/StyleMappingTokeniserTests.java
+++ b/src/test/java/org/zwobble/mammoth/tests/styles/parsing/StyleMappingTokeniserTests.java
@@ -47,16 +47,36 @@ public class StyleMappingTokeniserTests {
     }
 
     @Test
-    public void escapeSequencesInStringsAreTokenised() {
+    public void escapedStringTerminatorsInStringsAreTokenised() {
         assertTokens("'Tristan\\''", isToken(TokenType.STRING, "'Tristan\\''"));
     }
 
     @Test
+    public void escapeSequencesInStringsAreTokenised() {
+        assertTokens("'Tristan\\\\'", isToken(TokenType.STRING, "'Tristan\\\\'"));
+    }
+
+    @Test
     public void unterminatedStringsAreTokenised() {
         assertTokens("'Tristan", isToken(TokenType.UNTERMINATED_STRING, "'Tristan"));
     }
 
     @Test
+    public void unterminatedStringsEndingWithEscapedStringTerminatorAreTokenised() {
+        assertTokens("'Tristan\\'", isToken(TokenType.UNTERMINATED_STRING, "'Tristan\\'"));
+    }
+
+    @Test
+    public void unterminatedStringsWithUnterminatedEscapeAreTokenised() {
+        assertTokens("'Tristan\\", isToken(TokenType.UNTERMINATED_STRING, "'Tristan\\"));
+    }
+
+    @Test
+    public void unterminatedStringsWithManyEscapeSequencesAreTokenised() {
+        assertTokens("'" + repeatString("\\a", 50), isToken(TokenType.UNTERMINATED_STRING, "'" + repeatString("\\a", 50)));
+    }
+
+    @Test
     public void arrowsAreTokenised() {
         assertTokens("=>=>", isToken(TokenType.SYMBOL, "=>"), isToken(TokenType.SYMBOL, "=>"));
     }
@@ -138,4 +158,14 @@ public class StyleMappingTokeniserTests {
             hasProperty("value", equalTo(value))
         );
     }
+
+    private String repeatString(String value, int repeats) {
+        StringBuilder builder = new StringBuilder();
+
+        for (int i = 0; i < repeats; i++) {
+            builder.append(value);
+        }
+
+        return builder.toString();
+    }
 }