From ed0c81573526d6f4c84731d8f64ac84063d03cba Mon Sep 17 00:00:00 2001 From: John Keiser Date: Fri, 5 Jun 2020 12:36:38 -0700 Subject: [PATCH] Move unclosed array check to stage 2 --- src/arm64/dom_parser_implementation.cpp | 2 -- src/fallback/dom_parser_implementation.cpp | 2 -- src/generic/dom_parser_implementation.h | 20 -------------------- src/generic/stage2/structural_parser.h | 5 +++++ src/haswell/dom_parser_implementation.cpp | 2 -- src/westmere/dom_parser_implementation.cpp | 2 -- 6 files changed, 5 insertions(+), 28 deletions(-) diff --git a/src/arm64/dom_parser_implementation.cpp b/src/arm64/dom_parser_implementation.cpp index e676bcbb9..2e95cd683 100644 --- a/src/arm64/dom_parser_implementation.cpp +++ b/src/arm64/dom_parser_implementation.cpp @@ -112,8 +112,6 @@ namespace arm64 { WARN_UNUSED error_code dom_parser_implementation::parse(const uint8_t *_buf, size_t _len, dom::document &_doc) noexcept { error_code err = stage1(_buf, _len, false); if (err) { return err; } - err = check_for_unclosed_array(); - if (err) { return err; } return stage2(_doc); } diff --git a/src/fallback/dom_parser_implementation.cpp b/src/fallback/dom_parser_implementation.cpp index b472ec651..e77e5e976 100644 --- a/src/fallback/dom_parser_implementation.cpp +++ b/src/fallback/dom_parser_implementation.cpp @@ -238,8 +238,6 @@ namespace fallback { WARN_UNUSED error_code dom_parser_implementation::parse(const uint8_t *_buf, size_t _len, dom::document &_doc) noexcept { error_code err = stage1(_buf, _len, false); if (err) { return err; } - err = check_for_unclosed_array(); - if (err) { return err; } return stage2(_doc); } diff --git a/src/generic/dom_parser_implementation.h b/src/generic/dom_parser_implementation.h index 9c63405bb..4c8ec598f 100644 --- a/src/generic/dom_parser_implementation.h +++ b/src/generic/dom_parser_implementation.h @@ -31,7 +31,6 @@ public: WARN_UNUSED error_code parse(const uint8_t *buf, size_t len, dom::document &doc) noexcept final; WARN_UNUSED error_code stage1(const uint8_t *buf, size_t len, bool streaming) noexcept final; - WARN_UNUSED error_code check_for_unclosed_array() noexcept; WARN_UNUSED error_code stage2(dom::document &doc) noexcept final; WARN_UNUSED error_code stage2(const uint8_t *buf, size_t len, dom::document &doc, size_t &next_json) noexcept final; WARN_UNUSED error_code set_capacity(size_t capacity) noexcept final; @@ -57,22 +56,3 @@ WARN_UNUSED error_code dom_parser_implementation::set_max_depth(size_t max_depth _max_depth = max_depth; return SUCCESS; } - - -WARN_UNUSED error_code dom_parser_implementation::check_for_unclosed_array() noexcept { - // Before we engage stage 2, we want to make sure there is no risk that we could end with [ and - // loop back at the start with [. That is, we want to make sure that if the first character is [, then - // the last one is ]. - // See https://github.com/simdjson/simdjson/issues/906 for details. - if(n_structural_indexes < 2) { - return UNEXPECTED_ERROR; - } - const size_t first_index = structural_indexes[0]; - const size_t last_index = structural_indexes[n_structural_indexes - 2]; - const char first_character = char(buf[first_index]); - const char last_character = char(buf[last_index]); - if((first_character == '[') and (last_character != ']')) { - return TAPE_ERROR; - } - return SUCCESS; -} \ No newline at end of file diff --git a/src/generic/stage2/structural_parser.h b/src/generic/stage2/structural_parser.h index 85ba08367..80abcc46a 100644 --- a/src/generic/stage2/structural_parser.h +++ b/src/generic/stage2/structural_parser.h @@ -399,6 +399,11 @@ WARN_UNUSED error_code dom_parser_implementation::stage2(dom::document &_doc) no goto object_begin; case '[': FAIL_IF( parser.start_array(addresses.finish) ); + // Make sure the outer array is closed before continuing; otherwise, there are ways we could get + // into memory corruption. See https://github.com/simdjson/simdjson/issues/906 + if (buf[structural_indexes[n_structural_indexes - 2]] != ']') { + goto error; + } goto array_begin; case '"': FAIL_IF( parser.parse_string() ); diff --git a/src/haswell/dom_parser_implementation.cpp b/src/haswell/dom_parser_implementation.cpp index 44638df51..5377f7ebf 100644 --- a/src/haswell/dom_parser_implementation.cpp +++ b/src/haswell/dom_parser_implementation.cpp @@ -101,8 +101,6 @@ namespace haswell { WARN_UNUSED error_code dom_parser_implementation::parse(const uint8_t *_buf, size_t _len, dom::document &_doc) noexcept { error_code err = stage1(_buf, _len, false); if (err) { return err; } - err = check_for_unclosed_array(); - if (err) { return err; } return stage2(_doc); } diff --git a/src/westmere/dom_parser_implementation.cpp b/src/westmere/dom_parser_implementation.cpp index 56ac38881..7c5a5c852 100644 --- a/src/westmere/dom_parser_implementation.cpp +++ b/src/westmere/dom_parser_implementation.cpp @@ -102,8 +102,6 @@ namespace westmere { WARN_UNUSED error_code dom_parser_implementation::parse(const uint8_t *_buf, size_t _len, dom::document &_doc) noexcept { error_code err = stage1(_buf, _len, false); if (err) { return err; } - err = check_for_unclosed_array(); - if (err) { return err; } return stage2(_doc); }