Skip to content

Commit

Permalink
log: fix potential overflow with long log messages (#490)
Browse files Browse the repository at this point in the history
qb_vsnprintf_serialize was called with 'max_size' as the
limiting number for the length of the formatted log
message. But the buffer also needs to contain the
log header (given by 'actual_size'), so we now pass
't->max_line_length' as the maximum length of the
formatted log message to limit space to the actual 
bytes left

Also added error checks to the blackbox calls at
the end of the test, as these now provide a proper
test that the BB is functioning. Before they were
masking failures.
  • Loading branch information
chrissie-c authored Jul 20, 2023
1 parent 92ddd7c commit 1bbaa92
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 4 deletions.
4 changes: 2 additions & 2 deletions lib/log_blackbox.c
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ _blackbox_vlogger(int32_t target,
chunk += sizeof(uint32_t);

/* log message */
msg_len = qb_vsnprintf_serialize(chunk, max_size, cs->format, ap);
if (msg_len >= max_size) {
msg_len = qb_vsnprintf_serialize(chunk, t->max_line_length, cs->format, ap);
if (msg_len >= t->max_line_length) {
chunk = msg_len_pt + sizeof(uint32_t); /* Reset */

/* Leave this at QB_LOG_MAX_LEN so as not to overflow the blackbox */
Expand Down
6 changes: 4 additions & 2 deletions tests/check_log.c
Original file line number Diff line number Diff line change
Expand Up @@ -832,8 +832,10 @@ START_TEST(test_log_long_msg)
qb_log(LOG_INFO, "Message %d %d - %s", lpc, lpc%600, buffer);
}

qb_log_blackbox_write_to_file("blackbox.dump");
qb_log_blackbox_print_from_file("blackbox.dump");
rc = qb_log_blackbox_write_to_file("blackbox.dump");
ck_assert_int_gt(rc, 0);
rc = qb_log_blackbox_print_from_file("blackbox.dump");
ck_assert_int_le(rc, 0);
unlink("blackbox.dump");
qb_log_fini();
}
Expand Down

0 comments on commit 1bbaa92

Please sign in to comment.