From 43810a9548c6c9fe8ac30c4297714bf4539f6660 Mon Sep 17 00:00:00 2001 From: cherylEnkidu Date: Wed, 8 Jul 2026 12:13:18 -0400 Subject: [PATCH] Prevent crashes when parsing malformed bundle payloads --- .../core/src/bundle/bundle_serializer.cc | 11 +++++-- Firestore/core/src/util/json_reader.cc | 29 +++++++++++----- Firestore/core/src/util/json_reader.h | 4 +-- .../unit/bundle/bundle_serializer_test.cc | 33 +++++++++++++++++++ 4 files changed, 63 insertions(+), 14 deletions(-) diff --git a/Firestore/core/src/bundle/bundle_serializer.cc b/Firestore/core/src/bundle/bundle_serializer.cc index 86942236791..cf9769f2d7b 100644 --- a/Firestore/core/src/bundle/bundle_serializer.cc +++ b/Firestore/core/src/bundle/bundle_serializer.cc @@ -145,6 +145,10 @@ void DecodeCollectionSource(JsonReader& reader, const json& from_json, ResourcePath& parent, std::string& group) { + if (!from_json.is_array()) { + reader.Fail("'from' clause is not an array"); + return; + } const auto& from = from_json.get_ref&>(); if (from.size() != 1) { reader.Fail( @@ -274,9 +278,10 @@ int32_t DecodeLimit(JsonReader& reader, const json& query) { // "limit" can be encoded as integer or "{"value": integer}". if (limit_object.is_number_integer()) { return limit_object.get(); - } else if (limit_object.is_object()) { - if (limit_object.at("value").is_number_integer()) { - return limit_object.at("value").get(); + } else if (limit_object.is_object() && limit_object.contains("value")) { + const auto& limit_value = limit_object.at("value"); + if (limit_value.is_number_integer()) { + return limit_value.get(); } } reader.Fail("'limit' is not encoded as a valid integer"); diff --git a/Firestore/core/src/util/json_reader.cc b/Firestore/core/src/util/json_reader.cc index 6e719e8ff7e..eeb5add6dc4 100644 --- a/Firestore/core/src/util/json_reader.cc +++ b/Firestore/core/src/util/json_reader.cc @@ -38,7 +38,7 @@ const std::vector& EmptyVector() { const std::string& JsonReader::RequiredString(const char* name, const json& json_object) { - if (json_object.contains(name)) { + if (json_object.is_object() && json_object.contains(name)) { const json& child = json_object.at(name); if (child.is_string()) { return child.get_ref(); @@ -53,7 +53,7 @@ const std::string& JsonReader::OptionalString( const char* name, const json& json_object, const std::string& default_value) { - if (json_object.contains(name)) { + if (json_object.is_object() && json_object.contains(name)) { const json& child = json_object.at(name); if (child.is_string()) { return child.get_ref(); @@ -65,7 +65,7 @@ const std::string& JsonReader::OptionalString( const std::vector& JsonReader::RequiredArray(const char* name, const json& json_object) { - if (json_object.contains(name)) { + if (json_object.is_object() && json_object.contains(name)) { const json& child = json_object.at(name); if (child.is_array()) { return child.get_ref&>(); @@ -80,7 +80,7 @@ const std::vector& JsonReader::OptionalArray( const char* name, const json& json_object, const std::vector& default_value) { - if (!json_object.contains(name)) { + if (!json_object.is_object() || !json_object.contains(name)) { return default_value; } @@ -96,13 +96,21 @@ const std::vector& JsonReader::OptionalArray( bool JsonReader::OptionalBool(const char* name, const json& json_object, bool default_value) { - return (json_object.contains(name) && json_object.at(name).is_boolean() && - json_object.at(name).get()) || - default_value; + if (json_object.is_object() && json_object.contains(name)) { + const json& child = json_object.at(name); + if (child.is_boolean()) { + return child.get(); + } + } + return default_value; } const nlohmann::json& JsonReader::RequiredObject(const char* child_name, const json& json_object) { + if (!json_object.is_object()) { + Fail("Parent is not a JSON object"); + return json_object; + } if (!json_object.contains(child_name)) { Fail("Missing child '%s'", child_name); return json_object; @@ -114,6 +122,9 @@ const nlohmann::json& JsonReader::OptionalObject( const char* child_name, const json& json_object, const nlohmann::json& default_value) { + if (!json_object.is_object()) { + return default_value; + } if (json_object.contains(child_name)) { return json_object.at(child_name); } @@ -121,7 +132,7 @@ const nlohmann::json& JsonReader::OptionalObject( } double JsonReader::RequiredDouble(const char* name, const json& json_object) { - if (json_object.contains(name)) { + if (json_object.is_object() && json_object.contains(name)) { double result = DecodeDouble(json_object.at(name)); if (ok()) { return result; @@ -135,7 +146,7 @@ double JsonReader::RequiredDouble(const char* name, const json& json_object) { double JsonReader::OptionalDouble(const char* name, const json& json_object, double default_value) { - if (json_object.contains(name)) { + if (json_object.is_object() && json_object.contains(name)) { double result = DecodeDouble(json_object.at(name)); if (ok()) { return result; diff --git a/Firestore/core/src/util/json_reader.h b/Firestore/core/src/util/json_reader.h index f57c6aea9dd..8e704cc88ab 100644 --- a/Firestore/core/src/util/json_reader.h +++ b/Firestore/core/src/util/json_reader.h @@ -68,7 +68,7 @@ class JsonReader : public util::ReadContext { template IntType RequiredInt(const char* name, const nlohmann::json& json_object) { - if (!json_object.contains(name)) { + if (!json_object.is_object() || !json_object.contains(name)) { Fail("'%s' is missing or is not a double", name); return 0; } @@ -81,7 +81,7 @@ class JsonReader : public util::ReadContext { IntType OptionalInt(const char* name, const nlohmann::json& json_object, IntType default_value) { - if (!json_object.contains(name)) { + if (!json_object.is_object() || !json_object.contains(name)) { return default_value; } diff --git a/Firestore/core/test/unit/bundle/bundle_serializer_test.cc b/Firestore/core/test/unit/bundle/bundle_serializer_test.cc index 72d788c7832..079c5eaf539 100644 --- a/Firestore/core/test/unit/bundle/bundle_serializer_test.cc +++ b/Firestore/core/test/unit/bundle/bundle_serializer_test.cc @@ -1198,6 +1198,39 @@ TEST_F(BundleSerializerTest, EXPECT_EQ(core::LimitType::Last, named_query.bundled_query().limit_type()); } +TEST_F(BundleSerializerTest, DecodeCollectionSourceFromNotArrayFails) { + std::string json( + R"({"name":"myNamedQuery", +"bundledQuery":{"parent":"projects/p/databases/default/documents", +"structuredQuery":{"from":"not-an-array"},"limitType":"FIRST"}, +"readTime":{"seconds":"1679674432","nanos":579934000}})"); + JsonReader reader; + bundle_serializer.DecodeNamedQuery(reader, Parse(json)); + EXPECT_NOT_OK(reader.status()); +} + +TEST_F(BundleSerializerTest, DecodeLimitEmptyObjectFails) { + std::string json( + R"({"name":"myNamedQuery", +"bundledQuery":{"parent":"projects/p/databases/default/documents", +"structuredQuery":{"from":[{"collectionId":"foo"}], +"limit":{}},"limitType":"FIRST"}, +"readTime":{"seconds":"1679674432","nanos":579934000}})"); + JsonReader reader; + bundle_serializer.DecodeNamedQuery(reader, Parse(json)); + EXPECT_NOT_OK(reader.status()); +} + +TEST_F(BundleSerializerTest, DecodePrimitiveAsObjectFails) { + std::string json( + R"({"name":"myNamedQuery", +"bundledQuery":123, +"readTime":{"seconds":"1679674432","nanos":579934000}})"); + JsonReader reader; + bundle_serializer.DecodeNamedQuery(reader, Parse(json)); + EXPECT_NOT_OK(reader.status()); +} + } // namespace } // namespace bundle } // namespace firestore