Repository: maciejhirsz/json-rust (rust)

Issue:
Fails to correctly parse key with escaped forward slash
When attempting to parse the following string:

```
{"a\/b":"c\"d\"e","f":1}
```

The following value is created:

```
{
  "a/b": null,
  "c\"d": "c\"d\"e",
  "f": 1
}
```

It seems the combination of the escaped forward slash and the escaped quote in the value causes the parser to fail to attach the value in the correct location. 

Pull request description:
Fix #167
Fixes https://github.com/maciejhirsz/json-rust/issues/167 and prevents panics found in the `stringify` function when fuzzing with AFL. 

Also adds a number of tests to reproduce the fuzzing results and to test the changes to the parsing logic.

Patch:
diff --git a/Cargo.toml b/Cargo.toml
index c4759ca..95eff57 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "json"
-version = "0.12.0"
+version = "0.12.1"
 authors = ["Maciej Hirsz <hello@maciej.codes>"]
 description = "JSON implementation in Rust"
 repository = "https://github.com/maciejhirsz/json-rust"
diff --git a/src/codegen.rs b/src/codegen.rs
index dbda7a4..a0e7807 100644
--- a/src/codegen.rs
+++ b/src/codegen.rs
@@ -66,12 +66,12 @@ pub trait Generator {
 
     #[inline(never)]
     fn write_string_complex(&mut self, string: &str, mut start: usize) -> io::Result<()> {
-        self.write(string[ .. start].as_bytes())?;
+        self.write(&string.as_bytes()[ .. start])?;
 
         for (index, ch) in string.bytes().enumerate().skip(start) {
             let escape = ESCAPED[ch as usize];
             if escape > 0 {
-                self.write(string[start .. index].as_bytes())?;
+                self.write(&string.as_bytes()[start .. index])?;
                 self.write(&[b'\\', escape])?;
                 start = index + 1;
             }
@@ -79,7 +79,7 @@ pub trait Generator {
                 write!(self.get_writer(), "{:04x}", ch)?;
             }
         }
-        self.write(string[start ..].as_bytes())?;
+        self.write(&string.as_bytes()[start ..])?;
 
         self.write_char(b'"')
     }
@@ -382,3 +382,36 @@ fn extend_from_slice(dst: &mut Vec<u8>, src: &[u8]) {
             src_len);
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::borrow::Borrow;
+    use crate::JsonValue;
+    use crate::parse;
+
+  // found while fuzzing the DumpGenerator
+
+  #[test]
+  fn should_not_panic_on_bad_bytes() {
+    let data = [0, 12, 128, 88, 64, 99].to_vec();
+    let s = unsafe {
+        String::from_utf8_unchecked(data)
+    };
+
+    let mut generator = DumpGenerator::new();
+    generator.write_string(&s);
+  }
+
+  #[test]
+  fn should_not_panic_on_bad_bytes_2() {
+    let data = b"\x48\x48\x48\x57\x03\xE8\x48\x48\xE8\x03\x8F\x48\x29\x48\x48";
+    let s = unsafe {
+        String::from_utf8_unchecked(data.to_vec())
+    };
+
+    let mut generator = DumpGenerator::new();
+    generator.write_string(&s);
+  }
+
+}
\ No newline at end of file
diff --git a/src/parser.rs b/src/parser.rs
index 1c9ea7a..7fe8a07 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -483,11 +483,20 @@ impl<'a> Parser<'a> {
     // having to be read from source to a buffer and then from a buffer to
     // our target string. Nothing to be done about this, really.
     fn read_complex_string<'b>(&mut self, start: usize) -> Result<&'b str> {
-        self.buffer.clear();
+        // Since string slices are returned by this function that are created via pointers into `self.buffer`
+        // we shouldn't be clearing or modifying the buffer in consecutive calls to this function. Instead
+        // we continuously append bytes to `self.buffer` and keep track of the starting offset of the buffer on each
+        // call to this function. Later when creating string slices that point to the contents of this buffer
+        // we use this starting offset to make sure that newly created slices point only to the bytes that were
+        // appended in the most recent call to this function.
+        //
+        // Failing to do this can result in the StackBlock `key` values being modified in place later.
+        let len = self.buffer.len();
+        //self.buffer.clear();
         let mut ch = b'\\';
 
         // TODO: Use fastwrite here as well
-        self.buffer.extend_from_slice(self.source[start .. self.index - 1].as_bytes());
+        self.buffer.extend_from_slice(&self.source.as_bytes()[start .. self.index - 1]);
 
         loop {
             if ALLOWED[ch as usize] {
@@ -533,7 +542,7 @@ impl<'a> Parser<'a> {
                 // issues here, we construct a new slice from raw parts, which
                 // then has lifetime bound to the outer function scope instead
                 // of the parser itself.
-                slice::from_raw_parts(self.buffer.as_ptr(), self.buffer.len())
+                slice::from_raw_parts(self.buffer[len .. ].as_ptr(), self.buffer.len() - len)
             )
         })
     }
@@ -752,3 +761,138 @@ struct StackBlock(JsonValue, usize);
 pub fn parse(source: &str) -> Result<JsonValue> {
     Parser::new(source).parse()
 }
+
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::stringify;
+    use crate::JsonValue;
+
+    #[macro_use]
+    use crate::object;
+    use crate::array;
+
+    use std::fs::File;
+    use std::io::prelude::*;
+
+    #[test]
+    fn it_should_parse_escaped_forward_slashes_with_quotes() {
+        // used to get around the fact that rust strings don't escape forward slashes
+        let mut file = File::open("tests/test_json_slashes_quotes").unwrap();
+        let mut contents = String::new();
+        file.read_to_string(&mut contents).unwrap();
+
+        let actual = parse(&contents).unwrap();
+        let serialized = stringify(actual.clone());
+
+        assert_eq!(serialized, contents);
+    }
+
+    #[test]
+    fn it_should_parse_escaped_quotes() {
+        let contents = String::from("{\"ab\":\"c\\\"d\\\"e\"}");
+
+        let actual = parse(&contents).unwrap();
+        let serialized = stringify(actual.clone());
+
+        assert_eq!(serialized, contents);
+    }
+
+    #[test]
+    fn it_should_parse_basic_json_values() {
+        let s = "{\"a\":1,\"b\":true,\"c\":false,\"d\":null,\"e\":2}";
+        let actual = parse(s).unwrap();
+        let mut expected = object! {
+            "a" => 1,
+            "b" => true,
+            "c" => false,
+            "e" => 2
+        };
+        expected["d"] = JsonValue::Null;
+
+        assert_eq!(actual, expected);
+    }
+
+    #[test]
+    fn it_should_parse_json_arrays() {
+        let s = "{\"a\":1,\"b\":true,\"c\":false,\"d\":null,\"e\":2,\"f\":[1,2,3,false,true,[],{}]}";
+        let actual = parse(s).unwrap();
+        let mut expected = object! {
+            "a" => 1,
+            "b" => true,
+            "c" => false,
+            "e" => 2
+        };
+        expected["d"] = JsonValue::Null;
+        expected["f"] = array![
+            1,2,3,
+            false,
+            true,
+            array![],
+            object!{}
+        ];
+
+        assert_eq!(actual, expected);
+    }
+
+    #[test]
+    fn it_should_parse_json_nested_object() {
+        let s = "{\"a\":1,\"b\":{\"c\":2,\"d\":{\"e\":{\"f\":{\"g\":3,\"h\":[]}}},\"i\":4,\"j\":[],\"k\":{\"l\":5,\"m\":{}}}}";
+        let actual = parse(s).unwrap();
+        let mut expected = object! {
+            "a" => 1,
+            "b" => object!{
+                "c" => 2,
+                "d" => object!{
+                    "e" => object! {
+                        "f" => object!{
+                            "g" => 3,
+                            "h" => array![]
+                        }
+                    }
+                },
+                "i" => 4,
+                "j" => array![],
+                "k" => object!{
+                    "l" => 5,
+                    "m" => object!{}
+                }
+            }
+        };
+
+        assert_eq!(actual, expected);
+    }
+
+    #[test]
+    fn it_should_parse_json_complex_object() {
+        let s = "{\"a\":1,\"b\":{\"c\":2,\"d\":{\"e\":{\"f\":{\"g\":3,\"h\":[{\"z\":1},{\"y\":2,\"x\":[{},{}]}]}}},\"i\":4,\"j\":[],\"k\":{\"l\":5,\"m\":{}}}}";
+        let actual = parse(s).unwrap();
+        let mut expected = object! {
+            "a" => 1,
+            "b" => object!{
+                "c" => 2,
+                "d" => object!{
+                    "e" => object! {
+                        "f" => object!{
+                            "g" => 3,
+                            "h" => array![
+                                object!{"z" => 1},
+                                object!{"y" => 2, "x" => array![object!{}, object!{}]}
+                            ]
+                        }
+                    }
+                },
+                "i" => 4,
+                "j" => array![],
+                "k" => object!{
+                    "l" => 5,
+                    "m" => object!{}
+                }
+            }
+        };
+
+        assert_eq!(actual, expected);
+    }
+
+}
\ No newline at end of file


Test patch:
diff --git a/tests/test_json_slashes_quotes b/tests/test_json_slashes_quotes
new file mode 100644
index 0000000..514101a
--- /dev/null
+++ b/tests/test_json_slashes_quotes
@@ -0,0 +1,1 @@
+{"a\\/b":"c\"d\"e"}
\ No newline at end of file