Skip to content

Commit d541409

Browse files
Crypt-iQdergoeggestickies-v
committed
log: Add rate limiting to LogPrintf, LogInfo, LogWarning, LogError, LogPrintLevel
To mitigate disk-filling attacks caused by unsafe usages of LogPrintf and friends, we rate-limit them by passing a should_ratelimit bool that eventually makes its way to LogPrintStr which may call LogRateLimiter::Consume. The rate limiting is accomplished by adding a LogRateLimiter member to BCLog::Logger which tracks source code locations for the given logging window. Every hour, a source location can log up to 1MiB of data. Source locations that exceed the limit will have their logs suppressed for the rest of the window determined by m_limiter. This change affects the public LogPrintLevel function if called with a level >= BCLog::Level::Info. The UpdateTipLog function has been changed to use the private LogPrintLevel_ macro with should_ratelimit set to false. This allows UpdateTipLog to log during IBD without hitting the rate limit. Note that on restart, a source location that was rate limited before the restart will be able to log until it hits the rate limit again. Co-Authored-By: Niklas Gogge <[email protected]> Co-Authored-By: stickies-v <[email protected]>
1 parent a6a35cc commit d541409

File tree

5 files changed

+181
-31
lines changed

5 files changed

+181
-31
lines changed

src/init.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1378,6 +1378,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
13781378
}
13791379
}, std::chrono::minutes{5});
13801380

1381+
LogInstance().SetRateLimiting(std::make_unique<BCLog::LogRateLimiter>(
1382+
[&scheduler](auto func, auto window) { scheduler.scheduleEvery(std::move(func), window); },
1383+
BCLog::RATELIMIT_MAX_BYTES,
1384+
1h));
1385+
13811386
assert(!node.validation_signals);
13821387
node.validation_signals = std::make_unique<ValidationSignals>(std::make_unique<SerialTaskRunner>(scheduler));
13831388
auto& validation_signals = *node.validation_signals;

src/logging.cpp

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ bool BCLog::Logger::StartLogging()
7575
// dump buffered messages from before we opened the log
7676
m_buffering = false;
7777
if (m_buffer_lines_discarded > 0) {
78-
LogPrintStr_(strprintf("Early logging buffer overflowed, %d log lines discarded.\n", m_buffer_lines_discarded), std::source_location::current(), BCLog::ALL, Level::Info);
78+
LogPrintStr_(strprintf("Early logging buffer overflowed, %d log lines discarded.\n", m_buffer_lines_discarded), std::source_location::current(), BCLog::ALL, Level::Info, /*should_ratelimit=*/false);
7979
}
8080
while (!m_msgs_before_open.empty()) {
8181
const auto& buflog = m_msgs_before_open.front();
@@ -412,13 +412,14 @@ void BCLog::Logger::FormatLogStrInPlace(std::string& str, BCLog::LogFlags catego
412412
str.insert(0, LogTimestampStr(now, mocktime));
413413
}
414414

415-
void BCLog::Logger::LogPrintStr(std::string_view str, std::source_location&& source_loc, BCLog::LogFlags category, BCLog::Level level)
415+
void BCLog::Logger::LogPrintStr(std::string_view str, std::source_location&& source_loc, BCLog::LogFlags category, BCLog::Level level, bool should_ratelimit)
416416
{
417417
StdLockGuard scoped_lock(m_cs);
418-
return LogPrintStr_(str, std::move(source_loc), category, level);
418+
return LogPrintStr_(str, std::move(source_loc), category, level, should_ratelimit);
419419
}
420420

421-
void BCLog::Logger::LogPrintStr_(std::string_view str, std::source_location&& source_loc, BCLog::LogFlags category, BCLog::Level level)
421+
// NOLINTNEXTLINE(misc-no-recursion)
422+
void BCLog::Logger::LogPrintStr_(std::string_view str, std::source_location&& source_loc, BCLog::LogFlags category, BCLog::Level level, bool should_ratelimit)
422423
{
423424
std::string str_prefixed = LogEscapeMessage(str);
424425

@@ -451,6 +452,28 @@ void BCLog::Logger::LogPrintStr_(std::string_view str, std::source_location&& so
451452
}
452453

453454
FormatLogStrInPlace(str_prefixed, category, level, source_loc, util::ThreadGetInternalName(), SystemClock::now(), GetMockTime());
455+
bool ratelimit{false};
456+
if (should_ratelimit && m_limiter) {
457+
auto status{m_limiter->Consume(source_loc, str_prefixed)};
458+
if (status == BCLog::LogRateLimiter::Status::NEWLY_SUPPRESSED) {
459+
// NOLINTNEXTLINE(misc-no-recursion)
460+
LogPrintStr_(strprintf(
461+
"Excessive logging detected from %s:%d (%s): >%d bytes logged during "
462+
"the last time window of %is. Suppressing logging to disk from this "
463+
"source location until time window resets. Console logging "
464+
"unaffected. Last log entry.\n",
465+
source_loc.file_name(), source_loc.line(), source_loc.function_name(),
466+
m_limiter->m_max_bytes,
467+
Ticks<std::chrono::seconds>(m_limiter->m_reset_window)),
468+
std::source_location::current(), LogFlags::ALL, Level::Warning, /*should_ratelimit=*/false); // with should_ratelimit=false, this cannot lead to infinite recursion
469+
}
470+
ratelimit = status == BCLog::LogRateLimiter::Status::STILL_SUPPRESSED;
471+
// To avoid confusion caused by dropped log messages when debugging an issue,
472+
// we prefix log lines with "[*]" when there are any suppressed source locations.
473+
if (m_limiter->SuppressionsActive()) {
474+
str_prefixed.insert(0, "[*] ");
475+
}
476+
}
454477

455478
if (m_print_to_console) {
456479
// print to console
@@ -460,7 +483,7 @@ void BCLog::Logger::LogPrintStr_(std::string_view str, std::source_location&& so
460483
for (const auto& cb : m_print_callbacks) {
461484
cb(str_prefixed);
462485
}
463-
if (m_print_to_file) {
486+
if (m_print_to_file && !ratelimit) {
464487
assert(m_fileout != nullptr);
465488

466489
// reopen the log file, if requested
@@ -530,7 +553,7 @@ void BCLog::LogRateLimiter::Reset()
530553
uint64_t dropped_bytes{counter.GetDroppedBytes()};
531554
if (dropped_bytes == 0) continue;
532555
LogPrintLevel_(
533-
LogFlags::ALL, Level::Info,
556+
LogFlags::ALL, Level::Info, /*should_ratelimit=*/false,
534557
"Restarting logging from %s:%d (%s): %d bytes were dropped during the last %ss.\n",
535558
source_loc.file_name(), source_loc.line(), source_loc.function_name(),
536559
dropped_bytes, Ticks<std::chrono::seconds>(m_reset_window));

src/logging.h

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,9 @@ namespace BCLog {
200200
size_t m_cur_buffer_memusage GUARDED_BY(m_cs){0};
201201
size_t m_buffer_lines_discarded GUARDED_BY(m_cs){0};
202202

203+
//! Manages the rate limiting of each log location.
204+
std::unique_ptr<LogRateLimiter> m_limiter GUARDED_BY(m_cs);
205+
203206
//! Category-specific log level. Overrides `m_log_level`.
204207
std::unordered_map<LogFlags, Level> m_category_log_levels GUARDED_BY(m_cs);
205208

@@ -218,7 +221,7 @@ namespace BCLog {
218221
std::list<std::function<void(const std::string&)>> m_print_callbacks GUARDED_BY(m_cs) {};
219222

220223
/** Send a string to the log output (internal) */
221-
void LogPrintStr_(std::string_view str, std::source_location&& source_loc, BCLog::LogFlags category, BCLog::Level level)
224+
void LogPrintStr_(std::string_view str, std::source_location&& source_loc, BCLog::LogFlags category, BCLog::Level level, bool should_ratelimit)
222225
EXCLUSIVE_LOCKS_REQUIRED(m_cs);
223226

224227
std::string GetLogPrefix(LogFlags category, Level level) const;
@@ -237,7 +240,7 @@ namespace BCLog {
237240
std::atomic<bool> m_reopen_file{false};
238241

239242
/** Send a string to the log output */
240-
void LogPrintStr(std::string_view str, std::source_location&& source_loc, BCLog::LogFlags category, BCLog::Level level)
243+
void LogPrintStr(std::string_view str, std::source_location&& source_loc, BCLog::LogFlags category, BCLog::Level level, bool should_ratelimit)
241244
EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
242245

243246
/** Returns whether logs will be written to any output */
@@ -267,6 +270,12 @@ namespace BCLog {
267270
/** Only for testing */
268271
void DisconnectTestLogger() EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
269272

273+
void SetRateLimiting(std::unique_ptr<LogRateLimiter>&& limiter) EXCLUSIVE_LOCKS_REQUIRED(!m_cs)
274+
{
275+
StdLockGuard scoped_lock(m_cs);
276+
m_limiter = std::move(limiter);
277+
}
278+
270279
/** Disable logging
271280
* This offers a slight speedup and slightly smaller memory usage
272281
* compared to leaving the logging system in its default state.
@@ -334,7 +343,7 @@ static inline bool LogAcceptCategory(BCLog::LogFlags category, BCLog::Level leve
334343
bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str);
335344

336345
template <typename... Args>
337-
inline void LogPrintFormatInternal(std::source_location&& source_loc, const BCLog::LogFlags flag, const BCLog::Level level, util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
346+
inline void LogPrintFormatInternal(std::source_location&& source_loc, const BCLog::LogFlags flag, const BCLog::Level level, const bool should_ratelimit, util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
338347
{
339348
if (LogInstance().Enabled()) {
340349
std::string log_msg;
@@ -343,32 +352,38 @@ inline void LogPrintFormatInternal(std::source_location&& source_loc, const BCLo
343352
} catch (tinyformat::format_error& fmterr) {
344353
log_msg = "Error \"" + std::string{fmterr.what()} + "\" while formatting log message: " + fmt.fmt;
345354
}
346-
LogInstance().LogPrintStr(log_msg, std::move(source_loc), flag, level);
355+
LogInstance().LogPrintStr(log_msg, std::move(source_loc), flag, level, should_ratelimit);
347356
}
348357
}
349358

350-
#define LogPrintLevel_(category, level, ...) LogPrintFormatInternal(std::source_location::current(), category, level, __VA_ARGS__)
359+
#define LogPrintLevel_(category, level, should_ratelimit, ...) LogPrintFormatInternal(std::source_location::current(), category, level, should_ratelimit, __VA_ARGS__)
351360

352-
// Log unconditionally.
361+
// Log unconditionally. Uses basic rate limiting to mitigate disk filling attacks.
353362
// Be conservative when using functions that unconditionally log to debug.log!
354363
// It should not be the case that an inbound peer can fill up a user's storage
355364
// with debug.log entries.
356-
#define LogInfo(...) LogPrintLevel_(BCLog::LogFlags::ALL, BCLog::Level::Info, __VA_ARGS__)
357-
#define LogWarning(...) LogPrintLevel_(BCLog::LogFlags::ALL, BCLog::Level::Warning, __VA_ARGS__)
358-
#define LogError(...) LogPrintLevel_(BCLog::LogFlags::ALL, BCLog::Level::Error, __VA_ARGS__)
365+
#define LogInfo(...) LogPrintLevel_(BCLog::LogFlags::ALL, BCLog::Level::Info, /*should_ratelimit=*/true, __VA_ARGS__)
366+
#define LogWarning(...) LogPrintLevel_(BCLog::LogFlags::ALL, BCLog::Level::Warning, /*should_ratelimit=*/true, __VA_ARGS__)
367+
#define LogError(...) LogPrintLevel_(BCLog::LogFlags::ALL, BCLog::Level::Error, /*should_ratelimit=*/true, __VA_ARGS__)
359368

360369
// Deprecated unconditional logging.
361370
#define LogPrintf(...) LogInfo(__VA_ARGS__)
362371

363372
// Use a macro instead of a function for conditional logging to prevent
364373
// evaluating arguments when logging for the category is not enabled.
365374

366-
// Log conditionally, prefixing the output with the passed category name and severity level.
367-
#define LogPrintLevel(category, level, ...) \
368-
do { \
369-
if (LogAcceptCategory((category), (level))) { \
370-
LogPrintLevel_(category, level, __VA_ARGS__); \
371-
} \
375+
// Log by prefixing the output with the passed category name and severity level. This can either
376+
// log conditionally if the category is allowed or unconditionally if level >= BCLog::Level::Info
377+
// is passed. If this function logs unconditionally, logging to disk is rate-limited. This is
378+
// important so that callers don't need to worry about accidentally introducing a disk-fill
379+
// vulnerability if level >= Info is used. Additionally, users specifying -debug are assumed to be
380+
// developers or power users who are aware that -debug may cause excessive disk usage due to logging.
381+
#define LogPrintLevel(category, level, ...) \
382+
do { \
383+
if (LogAcceptCategory((category), (level))) { \
384+
bool rate_limit{level >= BCLog::Level::Info}; \
385+
LogPrintLevel_(category, level, rate_limit, __VA_ARGS__); \
386+
} \
372387
} while (0)
373388

374389
// Log conditionally, prefixing the output with the passed category name.

src/test/logging_tests.cpp

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <logging.h>
77
#include <logging/timer.h>
88
#include <scheduler.h>
9+
#include <test/util/logging.h>
910
#include <test/util/setup_common.h>
1011
#include <tinyformat.h>
1112
#include <util/fs.h>
@@ -15,8 +16,10 @@
1516
#include <chrono>
1617
#include <fstream>
1718
#include <future>
19+
#include <ios>
1820
#include <iostream>
1921
#include <source_location>
22+
#include <string>
2023
#include <unordered_map>
2124
#include <utility>
2225
#include <vector>
@@ -113,7 +116,7 @@ BOOST_FIXTURE_TEST_CASE(logging_LogPrintStr, LogSetup)
113116
std::vector<std::string> expected;
114117
for (auto& [msg, category, level, prefix, loc] : cases) {
115118
expected.push_back(tfm::format("[%s:%s] [%s] %s%s", util::RemovePrefix(loc.file_name(), "./"), loc.line(), loc.function_name(), prefix, msg));
116-
LogInstance().LogPrintStr(msg, std::move(loc), category, level);
119+
LogInstance().LogPrintStr(msg, std::move(loc), category, level, /*should_ratelimit=*/false);
117120
}
118121
std::ifstream file{tmp_log_path};
119122
std::vector<std::string> log_lines;
@@ -366,4 +369,106 @@ BOOST_AUTO_TEST_CASE(logging_log_limit_stats)
366369
BOOST_CHECK_EQUAL(counter.GetDroppedBytes(), 500ull);
367370
}
368371

372+
void LogFromLocation(int location, std::string message)
373+
{
374+
switch (location) {
375+
case 0:
376+
LogInfo("%s\n", message);
377+
break;
378+
case 1:
379+
LogInfo("%s\n", message);
380+
break;
381+
case 2:
382+
LogPrintLevel(BCLog::LogFlags::NONE, BCLog::Level::Info, "%s\n", message);
383+
break;
384+
case 3:
385+
LogPrintLevel(BCLog::LogFlags::ALL, BCLog::Level::Info, "%s\n", message);
386+
break;
387+
}
388+
}
389+
390+
void LogFromLocationAndExpect(int location, std::string message, std::string expect)
391+
{
392+
ASSERT_DEBUG_LOG(expect);
393+
LogFromLocation(location, message);
394+
}
395+
396+
BOOST_FIXTURE_TEST_CASE(logging_filesize_rate_limit, LogSetup)
397+
{
398+
bool prev_log_timestamps = LogInstance().m_log_timestamps;
399+
LogInstance().m_log_timestamps = false;
400+
bool prev_log_sourcelocations = LogInstance().m_log_sourcelocations;
401+
LogInstance().m_log_sourcelocations = false;
402+
bool prev_log_threadnames = LogInstance().m_log_threadnames;
403+
LogInstance().m_log_threadnames = false;
404+
405+
CScheduler scheduler{};
406+
scheduler.m_service_thread = std::thread([&] { scheduler.serviceQueue(); });
407+
auto sched_func = [&scheduler](auto func, auto window) { scheduler.scheduleEvery(std::move(func), window); };
408+
auto limiter = std::make_unique<BCLog::LogRateLimiter>(sched_func, 1024 * 1024, 20s);
409+
LogInstance().SetRateLimiting(std::move(limiter));
410+
411+
// Log 1024-character lines (1023 plus newline) to make the math simple.
412+
std::string log_message(1023, 'a');
413+
414+
std::string utf8_path{LogInstance().m_file_path.utf8string()};
415+
const char* log_path{utf8_path.c_str()};
416+
417+
// Use GetFileSize because fs::file_size may require a flush to be accurate.
418+
std::streamsize log_file_size{static_cast<std::streamsize>(GetFileSize(log_path))};
419+
420+
// Logging 1 MiB should be allowed.
421+
for (int i = 0; i < 1024; ++i) {
422+
LogFromLocation(0, log_message);
423+
}
424+
BOOST_CHECK_MESSAGE(log_file_size < GetFileSize(log_path), "should be able to log 1 MiB from location 0");
425+
426+
log_file_size = GetFileSize(log_path);
427+
428+
BOOST_CHECK_NO_THROW(LogFromLocationAndExpect(0, log_message, "Excessive logging detected"));
429+
BOOST_CHECK_MESSAGE(log_file_size < GetFileSize(log_path), "the start of the suppression period should be logged");
430+
431+
log_file_size = GetFileSize(log_path);
432+
for (int i = 0; i < 1024; ++i) {
433+
LogFromLocation(0, log_message);
434+
}
435+
436+
BOOST_CHECK_MESSAGE(log_file_size == GetFileSize(log_path), "all further logs from location 0 should be dropped");
437+
438+
BOOST_CHECK_THROW(LogFromLocationAndExpect(1, log_message, "Excessive logging detected"), std::runtime_error);
439+
BOOST_CHECK_MESSAGE(log_file_size < GetFileSize(log_path), "location 1 should be unaffected by other locations");
440+
441+
log_file_size = GetFileSize(log_path);
442+
{
443+
ASSERT_DEBUG_LOG("Restarting logging");
444+
MockForwardAndSync(scheduler, 1min);
445+
}
446+
447+
BOOST_CHECK_MESSAGE(log_file_size < GetFileSize(log_path), "the end of the suppression period should be logged");
448+
449+
BOOST_CHECK_THROW(LogFromLocationAndExpect(1, log_message, "Restarting logging"), std::runtime_error);
450+
451+
// Attempt to log 1MiB from location 2 and 1MiB from location 3. These exempt locations should be allowed to log
452+
// without limit.
453+
log_file_size = GetFileSize(log_path);
454+
for (int i = 0; i < 1024; ++i) {
455+
BOOST_CHECK_THROW(LogFromLocationAndExpect(2, log_message, "Excessive logging detected"), std::runtime_error);
456+
}
457+
458+
BOOST_CHECK_MESSAGE(log_file_size < GetFileSize(log_path), "location 2 should be exempt from rate limiting");
459+
460+
log_file_size = GetFileSize(log_path);
461+
for (int i = 0; i < 1024; ++i) {
462+
BOOST_CHECK_THROW(LogFromLocationAndExpect(3, log_message, "Excessive logging detected"), std::runtime_error);
463+
}
464+
465+
BOOST_CHECK_MESSAGE(log_file_size < GetFileSize(log_path), "location 3 should be exempt from rate limiting");
466+
467+
LogInstance().m_log_timestamps = prev_log_timestamps;
468+
LogInstance().m_log_sourcelocations = prev_log_sourcelocations;
469+
LogInstance().m_log_threadnames = prev_log_threadnames;
470+
scheduler.stop();
471+
LogInstance().SetRateLimiting(nullptr);
472+
}
473+
369474
BOOST_AUTO_TEST_SUITE_END()

src/validation.cpp

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2976,15 +2976,17 @@ static void UpdateTipLog(
29762976
{
29772977

29782978
AssertLockHeld(::cs_main);
2979-
LogPrintf("%s%s: new best=%s height=%d version=0x%08x log2_work=%f tx=%lu date='%s' progress=%f cache=%.1fMiB(%utxo)%s\n",
2980-
prefix, func_name,
2981-
tip->GetBlockHash().ToString(), tip->nHeight, tip->nVersion,
2982-
log(tip->nChainWork.getdouble()) / log(2.0), tip->m_chain_tx_count,
2983-
FormatISO8601DateTime(tip->GetBlockTime()),
2984-
chainman.GuessVerificationProgress(tip),
2985-
coins_tip.DynamicMemoryUsage() * (1.0 / (1 << 20)),
2986-
coins_tip.GetCacheSize(),
2987-
!warning_messages.empty() ? strprintf(" warning='%s'", warning_messages) : "");
2979+
2980+
// Disable rate limiting in LogPrintLevel_ so this source location may log during IBD.
2981+
LogPrintLevel_(BCLog::LogFlags::ALL, BCLog::Level::Info, /*should_ratelimit=*/false, "%s%s: new best=%s height=%d version=0x%08x log2_work=%f tx=%lu date='%s' progress=%f cache=%.1fMiB(%utxo)%s\n",
2982+
prefix, func_name,
2983+
tip->GetBlockHash().ToString(), tip->nHeight, tip->nVersion,
2984+
log(tip->nChainWork.getdouble()) / log(2.0), tip->m_chain_tx_count,
2985+
FormatISO8601DateTime(tip->GetBlockTime()),
2986+
chainman.GuessVerificationProgress(tip),
2987+
coins_tip.DynamicMemoryUsage() * (1.0 / (1 << 20)),
2988+
coins_tip.GetCacheSize(),
2989+
!warning_messages.empty() ? strprintf(" warning='%s'", warning_messages) : "");
29882990
}
29892991

29902992
void Chainstate::UpdateTip(const CBlockIndex* pindexNew)

0 commit comments

Comments
 (0)