Skip to content

Commit f1edfa8

Browse files
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 <[email protected]>
1 parent f1c5d53 commit f1edfa8

File tree

1 file changed

+74
-32
lines changed

1 file changed

+74
-32
lines changed

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

Lines changed: 74 additions & 32 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

@@ -403,52 +404,93 @@ void zwave_tx_queue::log_element(const zwave_tx_session_id_t session_id,
403404
void zwave_tx_queue::simple_log(zwave_tx_queue_element_t *e) const
404405
{
405406
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);
407+
int written = snprintf(message + index,
408+
sizeof(message) - index,
409+
"Enqueuing new frame (id=%p)",
410+
e->zwave_tx_session_id);
411+
if (written < 0 || written >= static_cast<int>(sizeof(message) - index)) {
412+
assert(false);
413+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::simple_log\n");
414+
return;
415+
}
416+
index += written;
410417

411418
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);
419+
written = snprintf(message + index,
420+
sizeof(message) - index,
421+
" (parent id=%p)",
422+
e->options.transport.parent_session_id);
423+
if (written < 0 || written >= static_cast<int>(sizeof(message) - index)) {
424+
assert(false);
425+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::simple_log\n");
426+
return;
427+
}
428+
index += written;
416429
}
417430

418431
// 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);
432+
written = snprintf(message + index,
433+
sizeof(message) - index,
434+
" - %d:%d -> ",
435+
e->connection_info.local.node_id,
436+
e->connection_info.local.endpoint_id);
437+
if (written < 0 || written >= static_cast<int>(sizeof(message) - index)) {
438+
assert(false);
439+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::simple_log\n");
440+
return;
441+
}
442+
index += written;
424443

425444
// Destination
426445
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);
446+
written = snprintf(message + index,
447+
sizeof(message) - index,
448+
" %d:%d - ",
449+
e->connection_info.remote.node_id,
450+
e->connection_info.remote.endpoint_id);
451+
if (written < 0 || written >= static_cast<int>(sizeof(message) - index)) {
452+
assert(false);
453+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::simple_log\n");
454+
return;
455+
}
456+
index += written;
457+
432458
} 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);
459+
written = snprintf(message + index,
460+
sizeof(message) - index,
461+
"Group ID %d (endpoint=%d) - ",
462+
e->connection_info.remote.multicast_group,
463+
e->connection_info.remote.endpoint_id);
464+
if (written < 0 || written >= static_cast<int>(sizeof(message) - index)) {
465+
assert(false);
466+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::simple_log\n");
467+
return;
468+
}
469+
index += written;
438470
}
439471

440472
// 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);
473+
written = snprintf(message + index,
474+
sizeof(message) - index,
475+
"Encapsulation %d - Payload (%d bytes) [",
476+
e->connection_info.encapsulation,
477+
e->data_length);
478+
if (written < 0 || written >= static_cast<int>(sizeof(message) - index)) {
479+
assert(false);
480+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::simple_log\n");
481+
return;
482+
}
483+
index += written;
446484

447485
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]);
486+
written
487+
= snprintf(message + index, sizeof(message) - index, "%02X ", e->data[i]);
488+
if (written < 0 || written >= static_cast<int>(sizeof(message) - index)) {
489+
assert(false);
490+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::simple_log\n");
491+
return;
492+
}
493+
index += written;
452494
}
453495
sl_log_debug(LOG_TAG, "%s] - Tx Queue size: %d\n", message, queue.size());
454496
}

0 commit comments

Comments
 (0)