Skip to content

Commit 272108f

Browse files
authored
Fix: Check flatbuffer integrity before parsing (#1864)
* Fix: Check flatbuffer integrity before parsing Updated flatbuffer to latest version to get verify buffer Use strol for key parsing to ensure exceptions do not result in a crash. * Handle new FBT_MAX_TYPE in flatbuffers * Fix the GenerateText response * Patch flatbuffers to resolve ERROR macro conflict on Windows * Add release notes * Restore 0001-remove-unused-var.patch for Android flatbuffers v1.12.0 build * Use cp -RL on Windows to avoid symbolic link creation failure * Address code review feedback on remote_config desktop implementation * Revert minor whitespace change in 0001-remove-unused-var.patch * Add default case to FlexbufferToVariant switch to handle unknown/corrupt types * Fix incorrect GenerateText return value comparison to nullptr * Add default case to FlexbufferToVariant switches in app and database modules * Avoid leading slash in app_data_prefix when package_name is empty/null * Add defensive null check for configs in RemoteConfigFileManager::Load * Fix logic inversion in RequestJson serialization assertion * Pin flatbuffers to official v25.12.19 release commit 7e163021 * Unify C++ and Java FlatBuffers versions to v25.2.10 * Update FlatBuffers patch for v25.2.10 formatting compatibility on Windows * Update Release Notes in readme.md for FlatBuffers v25.2.10 upgrade
1 parent 2669482 commit 272108f

13 files changed

Lines changed: 113 additions & 58 deletions

File tree

app/rest/request_json.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ class RequestJson : public Request {
6666
// Generate JSON string.
6767
std::string json;
6868
bool generate_status =
69-
GenerateText(*parser_, builder.GetBufferPointer(), &json);
69+
GenerateText(*parser_, builder.GetBufferPointer(), &json) == nullptr;
7070
FIREBASE_ASSERT_RETURN_VOID(generate_status);
7171

7272
set_post_fields(json.c_str());

app/src/variant_util.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,11 @@ Variant FlexbufferToVariant(const flexbuffers::Reference& ref) {
250250
case flexbuffers::FBT_BLOB:
251251
LogError("Flexbuffers containing blobs are not supported.");
252252
break;
253+
case flexbuffers::FBT_MAX_TYPE:
254+
default:
255+
LogError("Unknown or unsupported flexbuffer type: %d",
256+
static_cast<int>(ref.GetType()));
257+
break;
253258
}
254259
return Variant::Null();
255260
}

build_scripts/android/build.sh

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,5 @@ if [[ $(uname) == "Linux" ]] || [[ $(uname) == "Darwin" ]]; then
8181
else
8282
# rsync has to be specifically installed on windows bash (including github runners)
8383
# Also, rsync with absolute destination path doesn't work on Windows.
84-
# Using a simple copy instead of rsync on Windows.
85-
cp -R --parents "${paths[@]}" "${absbuildpath}"
84+
cp -RL --parents "${paths[@]}" "${absbuildpath}"
8685
fi

cmake/external/flatbuffers.cmake

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@ if(TARGET flatbuffers OR NOT DOWNLOAD_FLATBUFFERS)
1818
return()
1919
endif()
2020

21-
set(version 99aa1ef21dd9dc3f9d4fb0eb82f4b59d0bb5e4c5)
21+
# Pinned to the official v25.2.10 release commit to ensure stability and match Java library.
22+
set(version 1c514626e83c20fffa8557e75641848e1e15cd5e)
2223
set(patch_file
23-
${CMAKE_CURRENT_LIST_DIR}/../../scripts/git/patches/flatbuffers/0001-remove-unused-var.patch)
24+
${CMAKE_CURRENT_LIST_DIR}/../../scripts/git/patches/flatbuffers/0001-fix-error-macro.patch)
2425

2526
ExternalProject_Add(
2627
flatbuffers
@@ -29,7 +30,7 @@ ExternalProject_Add(
2930
COMMAND git init flatbuffers
3031
COMMAND cd flatbuffers && git fetch --depth=1 https://github.com/google/flatbuffers.git ${version} && git reset --hard FETCH_HEAD
3132

32-
PATCH_COMMAND git apply ${patch_file} && git gc --aggressive
33+
PATCH_COMMAND git apply ${patch_file}
3334
PREFIX ${PROJECT_BINARY_DIR}
3435

3536
CONFIGURE_COMMAND ""

database/src/desktop/persistence/flatbuffer_conversions.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,11 @@ Variant FlexbufferToVariant(const flexbuffers::Reference& ref) {
103103
case flexbuffers::FBT_BLOB:
104104
LogError("Flexbuffers containing blobs are not supported.");
105105
break;
106+
case flexbuffers::FBT_MAX_TYPE:
107+
default:
108+
LogError("Unknown or unsupported flexbuffer type: %d",
109+
static_cast<int>(ref.GetType()));
110+
break;
106111
}
107112
return Variant::Null();
108113
}

messaging/messaging_java/build.gradle

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,7 @@ buildscript {
2020
dependencies {
2121
classpath 'com.android.tools.build:gradle:7.4.2'
2222
classpath 'com.google.gms:google-services:4.4.1'
23-
// This uses Flatbuffers at 1.9 because the 1.10 version added a feature
24-
// that requires using a newer version of the JDK and at least Android N.
25-
// This has already been fixed at head, but a tagged release is not yet
26-
// available with that fix.
27-
classpath 'com.google.flatbuffers:flatbuffers-java:1.12.0'
23+
classpath 'com.google.flatbuffers:flatbuffers-java:25.2.10'
2824
}
2925
}
3026
allprojects {
@@ -64,7 +60,7 @@ dependencies {
6460
implementation platform('com.google.firebase:firebase-bom:34.15.0')
6561
implementation 'com.google.firebase:firebase-analytics'
6662
implementation 'com.google.firebase:firebase-messaging'
67-
implementation 'com.google.flatbuffers:flatbuffers-java:1.12.0'
63+
implementation 'com.google.flatbuffers:flatbuffers-java:25.2.10'
6864
}
6965

7066
afterEvaluate {
@@ -81,20 +77,13 @@ afterEvaluate {
8177
executable 'git'
8278
args 'clone',
8379
'--branch',
84-
'v1.12.0',
80+
'v25.2.10',
8581
'--depth',
8682
'1',
8783
'https://github.com/google/flatbuffers.git',
8884
flatbuffersDir
8985
}
90-
exec {
91-
executable 'git'
92-
args 'apply',
93-
'../../scripts/git/patches/flatbuffers/0001-remove-unused-var.patch',
94-
'--verbose',
95-
'--directory',
96-
'messaging/messaging_java/build/flatbuffers'
97-
}
86+
9887
}
9988

10089
// Locate or build flatc.

release_build_files/readme.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,11 @@ workflow use only during the development of your app, not for publicly shipping
613613
code.
614614

615615
## Release Notes
616+
### Upcoming
617+
- Changes
618+
- General: Upgraded FlatBuffers dependency to v25.2.10 across both native C++ and Java/Android components.
619+
- Remote Config (Desktop): Added flexbuffer integrity checks before parsing to prevent crashes on invalid/malformed data.
620+
616621
### 13.9.0
617622
- Changes
618623
- General (Android): Update to Firebase Android BoM version 34.15.0.

remote_config/src/desktop/config_data.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ std::string NamespacedConfigData::Serialize() const {
5151
void NamespacedConfigData::Deserialize(const std::string& buffer) {
5252
const uint8_t* data = reinterpret_cast<const uint8_t*>(buffer.data());
5353
size_t size = buffer.size();
54+
if (!flexbuffers::VerifyBuffer(data, size)) {
55+
return;
56+
}
5457
auto struct_map = flexbuffers::GetRoot(data, size).AsMap();
5558
flexbuffers::Map ns_config_map = struct_map["config_"].AsMap();
5659
for (int i = 0, in = ns_config_map.size(); i < in; ++i) {
@@ -144,6 +147,9 @@ std::string LayeredConfigs::Serialize() const {
144147
void LayeredConfigs::Deserialize(const std::string& buffer) {
145148
const uint8_t* data = reinterpret_cast<const uint8_t*>(buffer.data());
146149
size_t size = buffer.size();
150+
if (!flexbuffers::VerifyBuffer(data, size)) {
151+
return;
152+
}
147153
auto struct_map = flexbuffers::GetRoot(data, size).AsMap();
148154
fetched.Deserialize(struct_map["fetched"].AsString().str());
149155
active.Deserialize(struct_map["active"].AsString().str());

remote_config/src/desktop/file_manager.cc

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,18 @@ namespace internal {
3636

3737
RemoteConfigFileManager::RemoteConfigFileManager(const std::string& filename,
3838
const firebase::App& app) {
39+
const char* package_name = app.options().package_name();
3940
std::string app_data_prefix =
40-
std::string(app.options().package_name()) + "/" + app.name();
41-
std::string file_path =
42-
AppDataDir(app_data_prefix.c_str(), /*should_create=*/true) + "/" +
43-
filename;
41+
(package_name && package_name[0] != '\0')
42+
? std::string(package_name) + "/remote_config"
43+
: "remote_config";
44+
std::string error;
45+
std::string app_dir =
46+
AppDataDir(app_data_prefix.c_str(), /*should_create=*/true, &error);
47+
std::string file_path;
48+
if (error.empty() && !app_dir.empty()) {
49+
file_path = app_dir + "/" + app.name() + "_" + filename;
50+
}
4451
#if FIREBASE_PLATFORM_WINDOWS
4552
std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>> utf8_to_wstring;
4653
file_path_ = utf8_to_wstring.from_bytes(file_path);
@@ -50,16 +57,28 @@ RemoteConfigFileManager::RemoteConfigFileManager(const std::string& filename,
5057
}
5158

5259
bool RemoteConfigFileManager::Load(LayeredConfigs* configs) const {
60+
if (!configs || file_path_.empty()) {
61+
return false;
62+
}
5363
std::fstream input(file_path_, std::ios::in | std::ios::binary);
64+
if (!input) {
65+
return false;
66+
}
5467
std::stringstream ss;
5568
ss << input.rdbuf();
5669
configs->Deserialize(ss.str());
5770
return true;
5871
}
5972

6073
bool RemoteConfigFileManager::Save(const LayeredConfigs& configs) const {
74+
if (file_path_.empty()) {
75+
return false;
76+
}
6177
std::string buffer = configs.Serialize();
6278
std::fstream output(file_path_, std::ios::out | std::ios::binary);
79+
if (!output) {
80+
return false;
81+
}
6382
output.write(buffer.c_str(), buffer.size());
6483
return true;
6584
}

remote_config/src/desktop/metadata.cc

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@
1414

1515
#include "remote_config/src/desktop/metadata.h"
1616

17+
#include <cerrno>
1718
#include <cstdint>
19+
#include <cstdlib>
20+
#include <limits>
1821
#include <map>
1922
#include <string>
2023

@@ -59,6 +62,9 @@ std::string RemoteConfigMetadata::Serialize() const {
5962
void RemoteConfigMetadata::Deserialize(const std::string& buffer) {
6063
const uint8_t* data = reinterpret_cast<const uint8_t*>(buffer.data());
6164
size_t size = buffer.size();
65+
if (!flexbuffers::VerifyBuffer(data, size)) {
66+
return;
67+
}
6268
auto struct_map = flexbuffers::GetRoot(data, size).AsMap();
6369

6470
flexbuffers::Map info = struct_map["info"].AsMap();
@@ -76,7 +82,19 @@ void RemoteConfigMetadata::Deserialize(const std::string& buffer) {
7682
settings_.clear();
7783
flexbuffers::Map settings = struct_map["settings"].AsMap();
7884
for (int i = 0, n = settings.size(); i < n; ++i) {
79-
int int_key = std::stoi(settings.Keys()[i].AsKey());
85+
const char* key_str = settings.Keys()[i].AsKey();
86+
if (!key_str) continue;
87+
char* endptr = nullptr;
88+
errno = 0;
89+
long raw_key = std::strtol(key_str, &endptr, 10);
90+
if (endptr == key_str || *endptr != '\0' || errno == ERANGE) {
91+
continue;
92+
}
93+
if (raw_key < std::numeric_limits<int>::min() ||
94+
raw_key > std::numeric_limits<int>::max()) {
95+
continue;
96+
}
97+
int int_key = static_cast<int>(raw_key);
8098
settings_[static_cast<ConfigSetting>(int_key)] =
8199
settings.Values()[i].AsString().c_str();
82100
}

0 commit comments

Comments
 (0)