Repository: tafia/quick-xml (rust)

Issue:
Do not use unchecked multiply
```
thread '<unnamed>' panicked at 'attempt to multiply with overflow', /home/linkmauve/data/cargo/registry/src/github.com-1ecc6299db9ec823/quick-xml-0.10.0/src/escape.rs:166:9
```

Here is the testcase generated by cargo-fuzz:
```
\x00\x00<\x00\x00\x0a>&#44444444401?#\x0a413518#\x0a\x0a\x0a;<:<)(<:\x0a\x0a\x0a\x0a;<:\x0a\x0a<:\x0a\x0a\x0a\x0a\x0a<\x00*\x00\x00\x00\x00
```

The `&#44444444401` part seems to be the issue, multiplying that by ten does not fit in an `u32`, causing the panic.

This bug has been found while fuzzing [xmpp-parsers](https://hg.linkmauve.fr/xmpp-parsers) which uses [minidom](https://gitlab.com/lumi/minidom-rs), you would benefit a lot from fuzzing quick-xml itself.

Pull request description:
Issue101
Closes #101 

Patch:
diff --git a/fuzz/.gitignore b/fuzz/.gitignore
index dfeb7db..572e03b 100644
--- a/fuzz/.gitignore
+++ b/fuzz/.gitignore
@@ -1,5 +1,4 @@
 
 target
-libfuzzer
 corpus
 artifacts
diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml
index ce585b8..b24211f 100644
--- a/fuzz/Cargo.toml
+++ b/fuzz/Cargo.toml
@@ -18,5 +18,5 @@ git = "https://github.com/rust-fuzz/libfuzzer-sys.git"
 members = ["."]
 
 [[bin]]
-name = "fuzzer_script_1"
-path = "fuzzers/fuzzer_script_1.rs"
+name = "fuzz_target_1"
+path = "fuzz_targets/fuzz_target_1.rs"
diff --git a/fuzz/fuzz_targets/fuzz_target_1.rs b/fuzz/fuzz_targets/fuzz_target_1.rs
new file mode 100644
index 0000000..102fc5f
--- /dev/null
+++ b/fuzz/fuzz_targets/fuzz_target_1.rs
@@ -0,0 +1,35 @@
+#![no_main]
+#[macro_use] extern crate libfuzzer_sys;
+extern crate quick_xml;
+
+use quick_xml::reader::Reader;
+use quick_xml::events::Event;
+use std::io::Cursor;
+
+fuzz_target!(|data: &[u8]| {
+    // fuzzed code goes here
+    let cursor = Cursor::new(data);
+    let mut reader = Reader::from_reader(cursor);
+    let mut buf = vec![];
+    loop {
+        match reader.read_event(&mut buf) {
+            Ok(Event::Start(ref e)) | Ok(Event::Empty(ref e))=> {
+                if e.unescaped(&reader).is_err() {
+                    break;
+                }
+                for a in e.attributes() {
+                    if a.and_then(|a| a.unescaped_value()).is_err() {
+                        break;
+                    }
+                }
+            }
+            Ok(Event::Text(ref e)) => {
+                if e.unescaped(&reader).is_err() {
+                    break;
+                }
+            }
+            Ok(Event::Eof) | Err(..) => break,
+            _ => buf.clear(),
+        }
+    }
+});
diff --git a/fuzz/fuzzers/fuzzer_script_1.rs b/fuzz/fuzzers/fuzzer_script_1.rs
deleted file mode 100644
index 336dda7..0000000
--- a/fuzz/fuzzers/fuzzer_script_1.rs
+++ /dev/null
@@ -1,21 +0,0 @@
-#![no_main]
-extern crate libfuzzer_sys;
-extern crate quick_xml;
-
-use quick_xml::reader::Reader;
-use std::io::Cursor;
-
-#[export_name="rust_fuzzer_test_input"]
-pub extern fn go(data: &[u8]) {
-    // fuzzed code goes here
-    let cursor = Cursor::new(data);
-    let mut reader = Reader::from_reader(cursor);
-    let mut buf = vec![];
-    loop {
-        match reader.read_event(&mut buf) {
-            Ok(quick_xml::events::Event::Eof) | Err(..) => break,
-            _ => buf.clear(),
-        }
-    }
-}
-
diff --git a/src/escape.rs b/src/escape.rs
index 5fac7eb..ee88935 100644
--- a/src/escape.rs
+++ b/src/escape.rs
@@ -147,6 +147,10 @@ fn push_utf8(buf: &mut Vec<u8>, code: u32) {
 }
 
 fn parse_hexadecimal(bytes: &[u8]) -> Result<u32> {
+    // maximum code is 0x10FFFF => 6 characters
+    if bytes.len() > 6 {
+        bail!("Cannot convert hexadecimal to utf8");
+    }
     let mut code = 0;
     for &b in bytes {
         code <<= 4;
@@ -161,6 +165,10 @@ fn parse_hexadecimal(bytes: &[u8]) -> Result<u32> {
 }
 
 fn parse_decimal(bytes: &[u8]) -> Result<u32> {
+    // maximum code is 0x10FFFF = 1114111 => 7 characters
+    if bytes.len() > 7 {
+        bail!("Cannot convert decimal to utf8");
+    }
     let mut code = 0;
     for &b in bytes {
         code *= 10;
diff --git a/src/events/attributes.rs b/src/events/attributes.rs
index 064f5a6..4d5938b 100644
--- a/src/events/attributes.rs
+++ b/src/events/attributes.rs
@@ -175,14 +175,15 @@ impl<'a> Iterator for Attributes<'a> {
             {
                 return Some(self.error("Attribute key cannot contain quote", start_key + i));
             }
-            if let Some(r) = self.consumed.iter().cloned().find(|ref r| {
-                &self.bytes[(**r).clone()] == &self.bytes[start_key..end_key]
-            }) {
+            if let Some(r) = self.consumed
+                .iter()
+                .cloned()
+                .find(|ref r| &self.bytes[(**r).clone()] == &self.bytes[start_key..end_key])
+            {
                 return Some(self.error(
                     format!(
                         "Duplicate attribute at position {} and {}",
-                        r.start,
-                        start_key
+                        r.start, start_key
                     ),
                     start_key,
                 ));
diff --git a/src/events/mod.rs b/src/events/mod.rs
index 5a02a2b..8f8fd28 100644
--- a/src/events/mod.rs
+++ b/src/events/mod.rs
@@ -151,18 +151,14 @@ impl<'a> BytesDecl<'a> {
     pub fn version(&self) -> Result<Cow<[u8]>> {
         match self.element.attributes().next() {
             Some(Err(e)) => Err(e),
-            Some(
-                Ok(Attribute {
-                    key: b"version",
-                    value: v,
-                }),
-            ) => Ok(v),
-            Some(Ok(a)) => Err(
-                format!(
-                    "XmlDecl must start with 'version' attribute, found {:?}",
-                    from_utf8(a.key)
-                ).into(),
-            ),
+            Some(Ok(Attribute {
+                key: b"version",
+                value: v,
+            })) => Ok(v),
+            Some(Ok(a)) => Err(format!(
+                "XmlDecl must start with 'version' attribute, found {:?}",
+                from_utf8(a.key)
+            ).into()),
             None => Err("XmlDecl must start with 'version' attribute, found none".into()),
         }
     }
diff --git a/src/reader.rs b/src/reader.rs
index 6113530..1f6d5c3 100644
--- a/src/reader.rs
+++ b/src/reader.rs
@@ -301,9 +301,9 @@ impl<B: BufRead> Reader<B> {
                     offset -= 1;
                 }
             }
-            Ok(Event::Comment(
-                BytesText::borrowed(&buf[buf_start + 3..len - 2]),
-            ))
+            Ok(Event::Comment(BytesText::borrowed(
+                &buf[buf_start + 3..len - 2],
+            )))
         } else if len >= buf_start + 8 {
             match &buf[buf_start + 1..buf_start + 8] {
                 b"[CDATA[" => {
@@ -320,9 +320,9 @@ impl<B: BufRead> Reader<B> {
                         }
                         len = buf.len();
                     }
-                    Ok(Event::CData(
-                        BytesText::borrowed(&buf[buf_start + 8..len - 2]),
-                    ))
+                    Ok(Event::CData(BytesText::borrowed(
+                        &buf[buf_start + 8..len - 2],
+                    )))
                 }
                 b"DOCTYPE" => {
                     let mut count = buf.iter().skip(buf_start).filter(|&&b| b == b'<').count();
@@ -343,9 +343,9 @@ impl<B: BufRead> Reader<B> {
                         }
                     }
                     let len = buf.len();
-                    Ok(Event::DocType(
-                        BytesText::borrowed(&buf[buf_start + 8..len]),
-                    ))
+                    Ok(Event::DocType(BytesText::borrowed(
+                        &buf[buf_start + 8..len],
+                    )))
                 }
                 _ => bail!("Only Comment, CDATA and DOCTYPE nodes can start with a '!'"),
             }


Test patch:
diff --git a/tests/test.rs b/tests/test.rs
index c5436ec..161e54d 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -360,3 +360,35 @@ fn test_issue94() {
         buf.clear();
     }
 }
+
+#[test]
+fn fuzz_101() {
+    let data: &[u8] = b"\x00\x00<\x00\x00\x0a>&#44444444401?#\x0a413518\
+                       #\x0a\x0a\x0a;<:<)(<:\x0a\x0a\x0a\x0a;<:\x0a\x0a\
+                       <:\x0a\x0a\x0a\x0a\x0a<\x00*\x00\x00\x00\x00";
+    let cursor = Cursor::new(data);
+    let mut reader = Reader::from_reader(cursor);
+    let mut buf = vec![];
+    loop {
+        match reader.read_event(&mut buf) {
+            Ok(Start(ref e)) | Ok(Empty(ref e))=> {
+                if e.unescaped().is_err() {
+                    break;
+                }
+                for a in e.attributes() {
+                    if a.ok().map_or(true, |a| a.unescaped_value().is_err()) {
+                        break;
+                    }
+                }
+            }
+            Ok(Text(ref e)) => {
+                if e.unescaped().is_err() {
+                    break;
+                }
+            }
+            Ok(Eof) | Err(..) => break,
+            _ => (),
+        }
+        buf.clear();
+    }
+}