Skip to content

Commit 7a7f326

Browse files
authored
Merge pull request #699 from frstrtr/sec/codeql-gmtime-pathinj
security: clear CodeQL findings (gmtime thread-safety + path-injection hardening)
2 parents 3f261dd + e910523 commit 7a7f326

3 files changed

Lines changed: 33 additions & 4 deletions

File tree

src/c2pool/merged/coin_peer_manager.hpp

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <cmath>
1818
#include <random>
1919
#include <algorithm>
20+
#include <cctype>
2021
#include <array>
2122
#include <sstream>
2223

@@ -698,7 +699,20 @@ class CoinPeerManager
698699
else
699700
dir = m_data_dir;
700701
std::filesystem::create_directories(dir);
701-
return (dir / ("peers_" + m_symbol + ".json")).string();
702+
// Defense-in-depth (CodeQL cpp/path-injection): m_symbol is a hardcoded
703+
// ticker literal at every production ctor site, but never let it become a
704+
// path component. Reject anything non-alphanumeric so no '/','\\','.' or
705+
// NUL can survive into the filename.
706+
std::string sym = m_symbol;
707+
const bool clean = !sym.empty() && sym.size() <= 16 &&
708+
std::all_of(sym.begin(), sym.end(),
709+
[](unsigned char c) { return std::isalnum(c) != 0; });
710+
if (!clean) {
711+
LOG_WARNING << "[PeerManager] non-alphanumeric coin symbol '"
712+
<< m_symbol << "' -> using peers_unknown.json";
713+
sym = "unknown";
714+
}
715+
return (dir / ("peers_" + sym + ".json")).string();
702716
}
703717

704718
void save_peers()

src/core/filesystem.hpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#include <string>
44
#include <filesystem>
5+
#include <cstdlib>
56

67
namespace core
78
{
@@ -11,10 +12,15 @@ namespace filesystem
1112

1213
inline std::filesystem::path config_path()
1314
{
15+
// getenv may return nullptr if the var is unset; std::filesystem::path(nullptr)
16+
// is UB. Fall back to the current directory so an unset HOME/APPDATA degrades
17+
// gracefully instead of crashing.
1418
#ifdef _WIN32
15-
return std::filesystem::path(std::getenv("APPDATA")) / "c2pool";
19+
const char* base = std::getenv("APPDATA");
20+
return std::filesystem::path(base ? base : ".") / "c2pool";
1621
#else // Linux + macOS
17-
return std::filesystem::path(std::getenv("HOME")) / ".c2pool";
22+
const char* base = std::getenv("HOME");
23+
return std::filesystem::path(base ? base : ".") / ".c2pool";
1824
#endif
1925
}
2026

src/core/log.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,16 @@ void Logger::init(const std::string& log_file_name, int rotation_size_mb, int ma
160160
if (!fec && sz > static_cast<uintmax_t>(rotation_size_mb) * 1024 * 1024) {
161161
std::time_t t = std::time(nullptr);
162162
char ts[32];
163-
std::strftime(ts, sizeof(ts), "%Y%m%d-%H%M%S", std::gmtime(&t));
163+
// Thread-safe gmtime: std::gmtime returns a pointer into a shared
164+
// static tm and is not reentrant (CodeQL cpp/potentially-dangerous-
165+
// function). Use the reentrant per-platform variant into a local tm.
166+
std::tm tmv{};
167+
#ifdef _WIN32
168+
gmtime_s(&tmv, &t); // MSVC secure-CRT: (tm*, time_t*)
169+
#else
170+
gmtime_r(&t, &tmv); // POSIX GCC/Clang: (time_t*, tm*)
171+
#endif
172+
std::strftime(ts, sizeof(ts), "%Y%m%d-%H%M%S", &tmv);
164173
std::filesystem::path rotated =
165174
rotated_dir / (debug_log.filename().string() + std::string(".") + ts);
166175
std::filesystem::rename(debug_log, rotated, fec);

0 commit comments

Comments
 (0)