Skip to content

Fix: Check flatbuffer integrity before parsing #1864

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

Open
AustinBenoit wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix: Check flatbuffer integrity before parsing #1864

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dab06ad
Fix: Check flatbuffer integrity before parsing
AustinBenoit Jun 3, 2026
414623b
Update remote_config/src/desktop/metadata.cc
AustinBenoit Jun 3, 2026
dbc0e3d
Handle new FBT_MAX_TYPE in flatbuffers
AustinBenoit Jun 4, 2026
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
4 changes: 4 additions & 0 deletions app/src/variant_util.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,10 @@ Variant FlexbufferToVariant(const flexbuffers::Reference& ref) {
case flexbuffers::FBT_BLOB:
LogError("Flexbuffers containing blobs are not supported.");
break;
case flexbuffers::FBT_MAX_TYPE:
LogError("Unknown or unsupported flexbuffer type: %d",
static_cast<int>(ref.GetType()));
break;
}
return Variant::Null();
}
Expand Down
6 changes: 2 additions & 4 deletions cmake/external/flatbuffers.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@ if(TARGET flatbuffers OR NOT DOWNLOAD_FLATBUFFERS)
return()
endif()

set(version 99aa1ef21dd9dc3f9d4fb0eb82f4b59d0bb5e4c5)
set(patch_file
${CMAKE_CURRENT_LIST_DIR}/../../scripts/git/patches/flatbuffers/0001-remove-unused-var.patch)
# Commit corresponds to Tag v25.12.19-2026-02-06-03fffb2
set(version 95fda8c23e6e30ebd975892b5fad01efa53e039c)

ExternalProject_Add(
flatbuffers
Expand All @@ -29,7 +28,6 @@ ExternalProject_Add(
COMMAND git init flatbuffers
COMMAND cd flatbuffers && git fetch --depth=1 https://github.com/google/flatbuffers.git ${version} && git reset --hard FETCH_HEAD

PATCH_COMMAND git apply ${patch_file} && git gc --aggressive
PREFIX ${PROJECT_BINARY_DIR}

CONFIGURE_COMMAND ""
Expand Down
4 changes: 4 additions & 0 deletions database/src/desktop/persistence/flatbuffer_conversions.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ Variant FlexbufferToVariant(const flexbuffers::Reference& ref) {
case flexbuffers::FBT_BLOB:
LogError("Flexbuffers containing blobs are not supported.");
break;
case flexbuffers::FBT_MAX_TYPE:
LogError("Unknown or unsupported flexbuffer type: %d",
static_cast<int>(ref.GetType()));
break;
}
return Variant::Null();
}
Expand Down
6 changes: 6 additions & 0 deletions remote_config/src/desktop/config_data.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ std::string NamespacedConfigData::Serialize() const {
void NamespacedConfigData::Deserialize(const std::string& buffer) {
const uint8_t* data = reinterpret_cast<const uint8_t*>(buffer.data());
size_t size = buffer.size();
if (!flexbuffers::VerifyBuffer(data, size)) {
return;
}
auto struct_map = flexbuffers::GetRoot(data, size).AsMap();
flexbuffers::Map ns_config_map = struct_map["config_"].AsMap();
for (int i = 0, in = ns_config_map.size(); i < in; ++i) {
Expand Down Expand Up @@ -142,6 +145,9 @@ std::string LayeredConfigs::Serialize() const {
void LayeredConfigs::Deserialize(const std::string& buffer) {
const uint8_t* data = reinterpret_cast<const uint8_t*>(buffer.data());
size_t size = buffer.size();
if (!flexbuffers::VerifyBuffer(data, size)) {
return;
}
auto struct_map = flexbuffers::GetRoot(data, size).AsMap();
fetched.Deserialize(struct_map["fetched"].AsString().str());
active.Deserialize(struct_map["active"].AsString().str());
Expand Down
24 changes: 20 additions & 4 deletions remote_config/src/desktop/file_manager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,14 @@ namespace internal {
RemoteConfigFileManager::RemoteConfigFileManager(const std::string& filename,
const firebase::App& app) {
std::string app_data_prefix =
std::string(app.options().package_name()) + "/" + app.name();
std::string file_path =
AppDataDir(app_data_prefix.c_str(), /*should_create=*/true) + "/" +
filename;
std::string(app.options().package_name()) + "/remote_config";
Comment on lines 39 to +40
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If app.options().package_name() returns nullptr, constructing a std::string directly from it will result in undefined behavior (typically a crash). It is safer to perform a null check before constructing the string.

Suggested change
std::string app_data_prefix =
std::string(app.options().package_name()) + "/" + app.name();
std::string file_path =
AppDataDir(app_data_prefix.c_str(), /*should_create=*/true) + "/" +
filename;
std::string(app.options().package_name()) + "/remote_config";
const char* package_name = app.options().package_name();
std::string app_data_prefix =
std::string(package_name ? package_name : "") + "/remote_config";

std::string error;
std::string app_dir =
AppDataDir(app_data_prefix.c_str(), /*should_create=*/true, &error);
std::string file_path;
if (error.empty() && !app_dir.empty()) {
file_path = app_dir + "/" + app.name() + "_" + filename;
}
#if FIREBASE_PLATFORM_WINDOWS
std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>> utf8_to_wstring;
file_path_ = utf8_to_wstring.from_bytes(file_path);
Expand All @@ -50,16 +54,28 @@ RemoteConfigFileManager::RemoteConfigFileManager(const std::string& filename,
}

bool RemoteConfigFileManager::Load(LayeredConfigs* configs) const {
if (file_path_.empty()) {
return false;
}
std::fstream input(file_path_, std::ios::in | std::ios::binary);
if (!input) {
return false;
}
std::stringstream ss;
ss << input.rdbuf();
configs->Deserialize(ss.str());
return true;
}

bool RemoteConfigFileManager::Save(const LayeredConfigs& configs) const {
if (file_path_.empty()) {
return false;
}
std::string buffer = configs.Serialize();
std::fstream output(file_path_, std::ios::out | std::ios::binary);
if (!output) {
return false;
}
output.write(buffer.c_str(), buffer.size());
return true;
}
Expand Down
19 changes: 18 additions & 1 deletion remote_config/src/desktop/metadata.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@

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

#include <cerrno>
#include <cstdlib>
#include <limits>
#include <map>
#include <string>
Comment thread
AustinBenoit marked this conversation as resolved.

Expand Down Expand Up @@ -58,6 +61,9 @@ std::string RemoteConfigMetadata::Serialize() const {
void RemoteConfigMetadata::Deserialize(const std::string& buffer) {
const uint8_t* data = reinterpret_cast<const uint8_t*>(buffer.data());
size_t size = buffer.size();
if (!flexbuffers::VerifyBuffer(data, size)) {
return;
}
auto struct_map = flexbuffers::GetRoot(data, size).AsMap();

flexbuffers::Map info = struct_map["info"].AsMap();
Expand All @@ -75,7 +81,18 @@ void RemoteConfigMetadata::Deserialize(const std::string& buffer) {
settings_.clear();
flexbuffers::Map settings = struct_map["settings"].AsMap();
for (int i = 0, n = settings.size(); i < n; ++i) {
int int_key = std::stoi(settings.Keys()[i].AsKey());
const char* key_str = settings.Keys()[i].AsKey();
if (!key_str) continue;
char* endptr = nullptr;
long raw_key = std::strtol(key_str, &endptr, 10);
if (endptr == key_str || *endptr != '\0') {
continue;
}
if (raw_key < std::numeric_limits<int>::min() ||
raw_key > std::numeric_limits<int>::max()) {
continue;
}
Comment on lines +86 to +94
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

On platforms where sizeof(long) == sizeof(int) (such as Windows, which uses the LLP64 model where long is 32-bit), std::strtol will overflow and return LONG_MAX or LONG_MIN for values outside the 32-bit signed integer range. In these cases, raw_key will be equal to INT_MAX or INT_MIN, and the range checks raw_key < std::numeric_limits<int>::min() and raw_key > std::numeric_limits<int>::max() will fail to detect the overflow.

To correctly detect overflow on all platforms, you should clear errno before calling std::strtol and check if errno == ERANGE afterwards.

Suggested change
char* endptr = nullptr;
long raw_key = std::strtol(key_str, &endptr, 10);
if (endptr == key_str || *endptr != '\0') {
continue;
}
if (raw_key < std::numeric_limits<int>::min() ||
raw_key > std::numeric_limits<int>::max()) {
continue;
}
char* endptr = nullptr;
errno = 0;
long raw_key = std::strtol(key_str, &endptr, 10);
if (endptr == key_str || *endptr != '\\0' || errno == ERANGE) {
continue;
}
if (raw_key < std::numeric_limits<int>::min() ||
raw_key > std::numeric_limits<int>::max()) {
continue;
}

int int_key = static_cast<int>(raw_key);
settings_[static_cast<ConfigSetting>(int_key)] =
settings.Values()[i].AsString().c_str();
}
Expand Down
4 changes: 4 additions & 0 deletions remote_config/src/desktop/remote_config_response.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ Variant FlexbufferToVariant(const flexbuffers::Reference& ref) {
case flexbuffers::FBT_BLOB:
LogError("Flexbuffers containing blobs are not supported.");
break;
case flexbuffers::FBT_MAX_TYPE:
LogError("Unknown or unsupported flexbuffer type: %d",
static_cast<int>(ref.GetType()));
break;
}
return Variant::Null();
}
Expand Down
32 changes: 0 additions & 32 deletions scripts/git/patches/flatbuffers/0001-remove-unused-var.patch
View file
Open in desktop

This file was deleted.

Loading