From 3b03efa922d43afc59044ca06cbb2c452d1b8422 Mon Sep 17 00:00:00 2001 From: austinbenoit Date: Tue, 27 Oct 2026 13:49:43 +0000 Subject: [PATCH 1/2] Fix potential buffer underflow when stripping trailing newlines Use std::string::back() and std::string::pop_back() (with a non-empty check) instead of indexing buf[strlen(buf) - 1] in app/src/locale.cc and testing/run_all_tests.cc, and close /etc/timezone in GetTimezone(). Resolves GitHub Code Scanning security alerts (Rule WS-CPP-00049): - https://github.com/firebase/firebase-cpp-sdk/security/code-scanning/2788 - https://github.com/firebase/firebase-cpp-sdk/security/code-scanning/2789 - https://github.com/firebase/firebase-cpp-sdk/security/code-scanning/2791 --- app/src/locale.cc | 16 ++++++++++------ release_build_files/readme.md | 5 +++++ testing/run_all_tests.cc | 13 ++++++++----- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/app/src/locale.cc b/app/src/locale.cc index d19d598db2..cf662fc510 100644 --- a/app/src/locale.cc +++ b/app/src/locale.cc @@ -228,14 +228,18 @@ std::string GetTimezone() { FILE* tz_file = fopen("/etc/timezone", "r"); if (tz_file) { const size_t kBufSize = 128; - char buf[kBufSize]; - if (fgets(buf, kBufSize, tz_file)) { + char buf[kBufSize] = {0}; + const bool read_ok = fgets(buf, kBufSize, tz_file) != nullptr; + fclose(tz_file); + if (read_ok) { // Remove a trailing '\n', if any. - size_t len = strlen(buf); - if (buf[len - 1] == '\n') { - buf[len - 1] = '\0'; + std::string tz(buf); + if (!tz.empty() && tz.back() == '\n') { + tz.pop_back(); + } + if (!tz.empty()) { + return tz; } - return std::string(buf); } } // If TZ environment variable is defined and not empty, use it, else use diff --git a/release_build_files/readme.md b/release_build_files/readme.md index e1b9847c6e..1d935876a7 100644 --- a/release_build_files/readme.md +++ b/release_build_files/readme.md @@ -613,6 +613,11 @@ workflow use only during the development of your app, not for publicly shipping code. ## Release Notes +### Upcoming +- Changes + - General (Linux Desktop): Fixed a potential 1-byte buffer underflow and file + descriptor leak when reading `/etc/timezone` in `GetTimezone()`. + ### 14.1.0 - Changes - General (Android): Update to Firebase Android BoM version 35.1.0. diff --git a/testing/run_all_tests.cc b/testing/run_all_tests.cc index 284f83ea47..86b4f497a2 100644 --- a/testing/run_all_tests.cc +++ b/testing/run_all_tests.cc @@ -21,6 +21,8 @@ #include #include +#include + #include "base/logging.h" #include "gtest/gtest.h" @@ -91,12 +93,13 @@ extern "C" JNIEXPORT jint JNICALL JNI_FUNCTION_NAME if (fp != nullptr) { LOG(INFO) << "Native test logs:"; fseek(fp, 0, SEEK_SET); - char log[256]; - while (!feof(fp)) { - if (fgets(log, 256, fp)) { - log[strlen(log) - 1] = '\0'; // Remove newline. - LOG(INFO) << log; + char log[256] = {0}; + while (fgets(log, sizeof(log), fp) != nullptr) { + std::string line(log); + if (!line.empty() && line.back() == '\n') { + line.pop_back(); // Remove newline. } + LOG(INFO) << line; } fclose(fp); } else { From ca82349c6e504c057e3ed184d1a0bd27ec10b6b4 Mon Sep 17 00:00:00 2001 From: austinbenoit Date: Wed, 28 Oct 2026 12:42:34 +0000 Subject: [PATCH 2/2] Use std::ifstream/std::getline instead of fgets into fixed buffers Replaces the C-style fgets + manual newline stripping in GetTimezone() and the Android test runner with std::getline, which has no fixed-size buffer, no index arithmetic, and closes the file via RAII. GetTimezone() now also trims a trailing '\r' from CRLF line endings. --- app/src/locale.cc | 25 ++++++++----------------- testing/run_all_tests.cc | 12 +++++------- 2 files changed, 13 insertions(+), 24 deletions(-) diff --git a/app/src/locale.cc b/app/src/locale.cc index cf662fc510..bdb85f5dcb 100644 --- a/app/src/locale.cc +++ b/app/src/locale.cc @@ -33,10 +33,9 @@ #define UCHAR_TYPE wchar_t #include #elif FIREBASE_PLATFORM_LINUX -#include - #include #include +#include #else #error "Unknown platform." #endif // platform selector @@ -225,21 +224,13 @@ std::string GetTimezone() { #elif FIREBASE_PLATFORM_LINUX // Ubuntu: Check /etc/timezone for the full time zone name. - FILE* tz_file = fopen("/etc/timezone", "r"); - if (tz_file) { - const size_t kBufSize = 128; - char buf[kBufSize] = {0}; - const bool read_ok = fgets(buf, kBufSize, tz_file) != nullptr; - fclose(tz_file); - if (read_ok) { - // Remove a trailing '\n', if any. - std::string tz(buf); - if (!tz.empty() && tz.back() == '\n') { - tz.pop_back(); - } - if (!tz.empty()) { - return tz; - } + std::ifstream tz_file("/etc/timezone"); + std::string tz; + if (std::getline(tz_file, tz)) { + // Trim trailing whitespace, including the '\r' of a CRLF line ending. + tz.erase(tz.find_last_not_of(" \t\r\n") + 1); + if (!tz.empty()) { + return tz; } } // If TZ environment variable is defined and not empty, use it, else use diff --git a/testing/run_all_tests.cc b/testing/run_all_tests.cc index 86b4f497a2..dd935b3312 100644 --- a/testing/run_all_tests.cc +++ b/testing/run_all_tests.cc @@ -21,6 +21,7 @@ #include #include +#include #include #include "base/logging.h" @@ -92,13 +93,10 @@ extern "C" JNIEXPORT jint JNICALL JNI_FUNCTION_NAME #ifdef OUTPUT_TO_INFO if (fp != nullptr) { LOG(INFO) << "Native test logs:"; - fseek(fp, 0, SEEK_SET); - char log[256] = {0}; - while (fgets(log, sizeof(log), fp) != nullptr) { - std::string line(log); - if (!line.empty() && line.back() == '\n') { - line.pop_back(); // Remove newline. - } + fflush(fp); + std::ifstream log_file(log_path); + std::string line; + while (std::getline(log_file, line)) { LOG(INFO) << line; } fclose(fp);