Repository: raphlinus/pulldown-cmark (rust)

Issue:
[fuzz result] invalid HTML entities are parsed
According to the spec:

> [Decimal numeric character references](https://spec.commonmark.org/0.30/#decimal-numeric-character-references) consist of `&#` + a string of 1–7 arabic digits + `;`. A numeric character reference is parsed as the corresponding Unicode character. Invalid Unicode code points will be replaced by the REPLACEMENT CHARACTER (`U+FFFD`). For security reasons, the code point `U+0000` will also be replaced by `U+FFFD`.

That implies that `&#00000000;`, which has eight zeroes, is invalid. Yet pulldown-cmark accepts it.

<details>

Events from pulldown-cmark:

```rust
"v&#00000000;v\n" -> [
  Start(Paragraph)
    Text(Boxed("v�v"))
  End(Paragraph)
]
```

Events from pandoc:

```rust
"v&#00000000;v\n" -> [
  Start(Paragraph)
    Text(Boxed("v&#00000000;v"))
  End(Paragraph)
]
```

Events from commonmark.js:

```rust
"v&#00000000;v\n" -> [
  Start(Paragraph)
    Text(Boxed("v&#00000000;v"))
  End(Paragraph)
]
```

</details>


Pull request:
Reject long decimal and hex entities, turn invalid ones into U+FFFD
Fixes #826 

Patch:
diff --git a/specs/regression.txt b/specs/regression.txt
index b2bc45b..c716bab 100644
--- a/specs/regression.txt
+++ b/specs/regression.txt
@@ -2204,3 +2204,23 @@ First ![^1][] Second
 <p>foo</p>
 </div>
 ````````````````````````````````
+
+ISSUE 826
+
+HTML entities can only have up to seven digits,
+or six hexits.
+
+```````````````````````````````` example
+&#00000000; &#x0000000;
+.
+<p>&amp;#00000000; &amp;#x0000000;</p>
+````````````````````````````````
+
+&#5307530; is invalid unicode. As described in the spec,
+invalid unicode is replaced with the unknown U+FFFD.
+
+```````````````````````````````` example
+&#5307530;
+.
+<p>�</p>
+````````````````````````````````
diff --git a/src/scanners.rs b/src/scanners.rs
index 625db73..056829a 100644
--- a/src/scanners.rs
+++ b/src/scanners.rs
@@ -715,7 +715,7 @@ pub(crate) fn scan_listitem(bytes: &[u8]) -> Option<(usize, u8, usize, usize)> {
     let (w, start) = match c {
         b'-' | b'+' | b'*' => (1, 0),
         b'0'..=b'9' => {
-            let (length, start) = parse_decimal(bytes);
+            let (length, start) = parse_decimal(bytes, 9);
             c = *bytes.get(length)?;
             if !(c == b'.' || c == b')') {
                 return None;
@@ -743,9 +743,10 @@ pub(crate) fn scan_listitem(bytes: &[u8]) -> Option<(usize, u8, usize, usize)> {
 }
 
 // returns (number of bytes, parsed decimal)
-fn parse_decimal(bytes: &[u8]) -> (usize, usize) {
+fn parse_decimal(bytes: &[u8], limit: usize) -> (usize, usize) {
     match bytes
         .iter()
+        .take(limit)
         .take_while(|&&b| is_digit(b))
         .try_fold((0, 0usize), |(count, acc), c| {
             let digit = usize::from(c - b'0');
@@ -763,8 +764,8 @@ fn parse_decimal(bytes: &[u8]) -> (usize, usize) {
 }
 
 // returns (number of bytes, parsed hex)
-fn parse_hex(bytes: &[u8]) -> (usize, usize) {
-    match bytes.iter().try_fold((0, 0usize), |(count, acc), c| {
+fn parse_hex(bytes: &[u8], limit: usize) -> (usize, usize) {
+    match bytes.iter().take(limit).try_fold((0, 0usize), |(count, acc), c| {
         let mut c = *c;
         let digit = if c >= b'0' && c <= b'9' {
             usize::from(c - b'0')
@@ -791,9 +792,9 @@ fn parse_hex(bytes: &[u8]) -> (usize, usize) {
 }
 
 fn char_from_codepoint(input: usize) -> Option<char> {
-    let mut codepoint = input.try_into().ok()?;
+    let codepoint = input.try_into().ok()?;
     if codepoint == 0 {
-        codepoint = 0xFFFD;
+        return None;
     }
     char::from_u32(codepoint)
 }
@@ -805,17 +806,15 @@ pub(crate) fn scan_entity(bytes: &[u8]) -> (usize, Option<CowStr<'static>>) {
         end += 1;
         let (bytecount, codepoint) = if end < bytes.len() && bytes[end] | 0x20 == b'x' {
             end += 1;
-            parse_hex(&bytes[end..])
+            parse_hex(&bytes[end..], 6)
         } else {
-            parse_decimal(&bytes[end..])
+            parse_decimal(&bytes[end..], 7)
         };
         end += bytecount;
         return if bytecount == 0 || scan_ch(&bytes[end..], b';') == 0 {
             (0, None)
-        } else if let Some(c) = char_from_codepoint(codepoint) {
-            (end + 1, Some(c.into()))
         } else {
-            (0, None)
+            (end + 1, Some(char_from_codepoint(codepoint).unwrap_or('\u{FFFD}').into()))
         };
     }
     end += scan_while(&bytes[end..], is_ascii_alphanumeric);


Test patch:
diff --git a/tests/suite/regression.rs b/tests/suite/regression.rs
index 4324178..d863a01 100644
--- a/tests/suite/regression.rs
+++ b/tests/suite/regression.rs
@@ -2571,7 +2571,7 @@ fn regression_test_159() {
 
     test_markdown_html(original, expected, false, false, false);
 }
-  
+
 #[test]
 fn regression_test_160() {
     let original = r##"![^1]
@@ -2601,3 +2601,23 @@ fn regression_test_161() {
 
     test_markdown_html(original, expected, false, false, false);
 }
+
+#[test]
+fn regression_test_162() {
+    let original = r##"&#00000000; &#x0000000;
+"##;
+    let expected = r##"<p>&amp;#00000000; &amp;#x0000000;</p>
+"##;
+
+    test_markdown_html(original, expected, false, false, false);
+}
+
+#[test]
+fn regression_test_163() {
+    let original = r##"&#5307530;
+"##;
+    let expected = r##"<p>�</p>
+"##;
+
+    test_markdown_html(original, expected, false, false, false);
+}