Skip to content

Prevent crashes when parsing malformed bundle payloads #16367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wu-hui merged 1 commit into from
Jul 13, 2026
+63 −14
Merged

Prevent crashes when parsing malformed bundle payloads #16367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions Firestore/core/src/bundle/bundle_serializer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<const std::vector<json>&>();
if (from.size() != 1) {
reader.Fail(
Expand Down Expand Up @@ -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<int32_t>();
} else if (limit_object.is_object()) {
if (limit_object.at("value").is_number_integer()) {
return limit_object.at("value").get<int32_t>();
} 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<int32_t>();
}
}
reader.Fail("'limit' is not encoded as a valid integer");
Expand Down
29 changes: 20 additions & 9 deletions Firestore/core/src/util/json_reader.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const std::vector<T>& 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<const std::string&>();
Expand All @@ -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<const std::string&>();
Expand All @@ -65,7 +65,7 @@ const std::string& JsonReader::OptionalString(

const std::vector<json>& 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<const std::vector<json>&>();
Expand All @@ -80,7 +80,7 @@ const std::vector<json>& JsonReader::OptionalArray(
const char* name,
const json& json_object,
const std::vector<json>& default_value) {
if (!json_object.contains(name)) {
if (!json_object.is_object() || !json_object.contains(name)) {
return default_value;
}

Expand All @@ -96,13 +96,21 @@ const std::vector<json>& 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<bool>()) ||
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<bool>();
}
}
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;
Expand All @@ -114,14 +122,17 @@ 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);
}
return default_value;
}

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;
Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions Firestore/core/src/util/json_reader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class JsonReader : public util::ReadContext {

template <typename IntType>
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;
}
Expand All @@ -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;
}

Expand Down
33 changes: 33 additions & 0 deletions Firestore/core/test/unit/bundle/bundle_serializer_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading