Skip to content

Commit 49326bd

Browse files
rzrgithub-advanced-security[bot]
rzr
and
github-advanced-security[bot]
committed
z_tx: Harden zwave_tx_queue.cpp by checking snprintf
Checking snprintf results, reminder : If the output was truncated due to this limit, then the return value is the number of characters (excluding the terminating null byte) which would have been written to the final string if enough space had been available This was found using CodeQL: Potential fix for code scanning alert no. 16: Potentially overflowing call to snprintf Relate-to: SiliconLabsSoftware#100 Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Philippe Coval <philippe.coval@silabs.com>
1 parent f1c5d53 commit 49326bd

File tree

1 file changed

+117
-46
lines changed

1 file changed

+117
-46
lines changed

‎applications/zpc/components/zwave/zwave_tx/src/zwave_tx_queue.cpp‎

Lines changed: 117 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "sl_log.h"
2222

2323
// Generic includes
24+
#include <assert.h>
2425
#include <string.h> // Using memcpy
2526
#include <cstdio> // snprintf
2627

@@ -290,6 +291,7 @@ bool zwave_tx_queue::zwave_tx_has_frames_for_node(const zwave_node_id_t node_id)
290291
void zwave_tx_queue::log(bool log_messages_payload) const
291292
{
292293
sl_log_debug(LOG_TAG, "Queue size: %lu\n", (unsigned long)queue.size());
294+
int written = 0;
293295
for (auto it = queue.begin(); it != queue.end(); ++it) {
294296
sl_log_debug(LOG_TAG,
295297
"Entry (id=%p): (address %p)\n",
@@ -337,15 +339,28 @@ void zwave_tx_queue::log(bool log_messages_payload) const
337339
it->transmission_time);
338340
if (true == log_messages_payload) {
339341
uint16_t index = 0;
340-
index += snprintf(message + index,
341-
sizeof(message) - index,
342-
"Frame payload (hex): ");
342+
written = snprintf(message + index,
343+
sizeof(message) - index,
344+
"Frame payload (hex): ");
345+
if (written < 0 || written >= static_cast<int>(sizeof(message) - index)) {
346+
assert(false);
347+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::log\n");
348+
return;
349+
}
350+
index += written;
343351

344352
for (uint16_t i = 0; i < it->data_length; i++) {
345-
index += snprintf(message + index,
346-
sizeof(message) - index,
347-
"%02X ",
348-
it->data[i]);
353+
written = snprintf(message + index,
354+
sizeof(message) - index,
355+
"%02X ",
356+
it->data[i]);
357+
if (written < 0
358+
|| written >= static_cast<int>(sizeof(message) - index)) {
359+
assert(false);
360+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::log\n");
361+
return;
362+
}
363+
index += written;
349364
}
350365
sl_log_debug(LOG_TAG, "\t%s\n", message);
351366
}
@@ -355,6 +370,7 @@ void zwave_tx_queue::log(bool log_messages_payload) const
355370
void zwave_tx_queue::log_element(const zwave_tx_session_id_t session_id,
356371
bool log_frame_payload) const
357372
{
373+
int written = 0;
358374
for (auto it = queue.begin(); it != queue.end(); ++it) {
359375
if (it->zwave_tx_session_id == session_id) {
360376
sl_log_debug(
@@ -382,15 +398,29 @@ void zwave_tx_queue::log_element(const zwave_tx_session_id_t session_id,
382398
it->transmission_time);
383399
if (true == log_frame_payload) {
384400
uint16_t index = 0;
385-
index += snprintf(message + index,
386-
sizeof(message) - index,
387-
"Frame payload (hex): ");
401+
written = snprintf(message + index,
402+
sizeof(message) - index,
403+
"Frame payload (hex): ");
404+
if (written < 0
405+
|| written >= static_cast<int>(sizeof(message) - index)) {
406+
assert(false);
407+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::log_element\n");
408+
return;
409+
}
410+
index += written;
388411

389412
for (uint16_t i = 0; i < it->data_length; i++) {
390-
index += snprintf(message + index,
391-
sizeof(message) - index,
392-
"%02X ",
393-
it->data[i]);
413+
written = snprintf(message + index,
414+
sizeof(message) - index,
415+
"%02X ",
416+
it->data[i]);
417+
if (written < 0
418+
|| written >= static_cast<int>(sizeof(message) - index)) {
419+
assert(false);
420+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::log_element\n");
421+
return;
422+
}
423+
index += written;
394424
}
395425
sl_log_debug(LOG_TAG, "%s\n", message);
396426
}
@@ -403,52 +433,93 @@ void zwave_tx_queue::log_element(const zwave_tx_session_id_t session_id,
403433
void zwave_tx_queue::simple_log(zwave_tx_queue_element_t *e) const
404434
{
405435
uint16_t index = 0;
406-
index += snprintf(message + index,
407-
sizeof(message) - index,
408-
"Enqueuing new frame (id=%p)",
409-
e->zwave_tx_session_id);
436+
int written = snprintf(message + index,
437+
sizeof(message) - index,
438+
"Enqueuing new frame (id=%p)",
439+
e->zwave_tx_session_id);
440+
if (written < 0 || written >= static_cast<int>(sizeof(message) - index)) {
441+
assert(false);
442+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::simple_log\n");
443+
return;
444+
}
445+
index += written;
410446

411447
if (e->options.transport.valid_parent_session_id == true) {
412-
index += snprintf(message + index,
413-
sizeof(message) - index,
414-
" (parent id=%p)",
415-
e->options.transport.parent_session_id);
448+
written = snprintf(message + index,
449+
sizeof(message) - index,
450+
" (parent id=%p)",
451+
e->options.transport.parent_session_id);
452+
if (written < 0 || written >= static_cast<int>(sizeof(message) - index)) {
453+
assert(false);
454+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::simple_log\n");
455+
return;
456+
}
457+
index += written;
416458
}
417459

418460
// Source address
419-
index += snprintf(message + index,
420-
sizeof(message) - index,
421-
" - %d:%d -> ",
422-
e->connection_info.local.node_id,
423-
e->connection_info.local.endpoint_id);
461+
written = snprintf(message + index,
462+
sizeof(message) - index,
463+
" - %d:%d -> ",
464+
e->connection_info.local.node_id,
465+
e->connection_info.local.endpoint_id);
466+
if (written < 0 || written >= static_cast<int>(sizeof(message) - index)) {
467+
assert(false);
468+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::simple_log\n");
469+
return;
470+
}
471+
index += written;
424472

425473
// Destination
426474
if (e->connection_info.remote.is_multicast == false) {
427-
index += snprintf(message + index,
428-
sizeof(message) - index,
429-
" %d:%d - ",
430-
e->connection_info.remote.node_id,
431-
e->connection_info.remote.endpoint_id);
475+
written = snprintf(message + index,
476+
sizeof(message) - index,
477+
" %d:%d - ",
478+
e->connection_info.remote.node_id,
479+
e->connection_info.remote.endpoint_id);
480+
if (written < 0 || written >= static_cast<int>(sizeof(message) - index)) {
481+
assert(false);
482+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::simple_log\n");
483+
return;
484+
}
485+
index += written;
486+
432487
} else {
433-
index += snprintf(message + index,
434-
sizeof(message) - index,
435-
"Group ID %d (endpoint=%d) - ",
436-
e->connection_info.remote.multicast_group,
437-
e->connection_info.remote.endpoint_id);
488+
written = snprintf(message + index,
489+
sizeof(message) - index,
490+
"Group ID %d (endpoint=%d) - ",
491+
e->connection_info.remote.multicast_group,
492+
e->connection_info.remote.endpoint_id);
493+
if (written < 0 || written >= static_cast<int>(sizeof(message) - index)) {
494+
assert(false);
495+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::simple_log\n");
496+
return;
497+
}
498+
index += written;
438499
}
439500

440501
// Encapsulation & payload
441-
index += snprintf(message + index,
442-
sizeof(message) - index,
443-
"Encapsulation %d - Payload (%d bytes) [",
444-
e->connection_info.encapsulation,
445-
e->data_length);
502+
written = snprintf(message + index,
503+
sizeof(message) - index,
504+
"Encapsulation %d - Payload (%d bytes) [",
505+
e->connection_info.encapsulation,
506+
e->data_length);
507+
if (written < 0 || written >= static_cast<int>(sizeof(message) - index)) {
508+
assert(false);
509+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::simple_log\n");
510+
return;
511+
}
512+
index += written;
446513

447514
for (uint16_t i = 0; i < e->data_length; i++) {
448-
index += snprintf(message + index,
449-
sizeof(message) - index,
450-
"%02X ",
451-
e->data[i]);
515+
written
516+
= snprintf(message + index, sizeof(message) - index, "%02X ", e->data[i]);
517+
if (written < 0 || written >= static_cast<int>(sizeof(message) - index)) {
518+
assert(false);
519+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::simple_log\n");
520+
return;
521+
}
522+
index += written;
452523
}
453524
sl_log_debug(LOG_TAG, "%s] - Tx Queue size: %d\n", message, queue.size());
454525
}

0 commit comments

Comments
 (0)