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:
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