Skip to content

Commit ee18ee9

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 <philippe.coval@silabs.com>
1 parent bb8ac35 commit ee18ee9

1 file changed

Lines changed: 79 additions & 32 deletions

File tree

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

Lines changed: 79 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -403,52 +403,99 @@ void zwave_tx_queue::log_element(const zwave_tx_session_id_t session_id,
403403
void zwave_tx_queue::simple_log(zwave_tx_queue_element_t *e) const
404404
{
405405
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);
406+
int written = snprintf(message + index,
407+
sizeof(message) - index,
408+
"Enqueuing new frame (id=%p)",
409+
e->zwave_tx_session_id);
410+
if (written < 0 || written >= static_cast<int>(sizeof(message) - index)) {
411+
assert(false);
412+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::simple_log\n");
413+
return;
414+
}
415+
index += written;
410416

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

425445
// Destination
426446
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);
447+
written = snprintf(message + index,
448+
sizeof(message) - index,
449+
" %d:%d - ",
450+
e->connection_info.remote.node_id,
451+
e->connection_info.remote.endpoint_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+
456+
return;
457+
}
458+
index += written;
459+
432460
} 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);
461+
written = snprintf(message + index,
462+
sizeof(message) - index,
463+
"Group ID %d (endpoint=%d) - ",
464+
e->connection_info.remote.multicast_group,
465+
e->connection_info.remote.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+
470+
return;
471+
}
472+
index += written;
438473
}
439474

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

447489
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]);
490+
written
491+
= snprintf(message + index, sizeof(message) - index, "%02X ", e->data[i]);
492+
if (written < 0 || written >= static_cast<int>(sizeof(message) - index)) {
493+
assert(false);
494+
sl_log_error(LOG_TAG, "Overflow in zwave_tx_queue::simple_log\n");
495+
496+
return;
497+
}
498+
index += written;
452499
}
453500
sl_log_debug(LOG_TAG, "%s] - Tx Queue size: %d\n", message, queue.size());
454501
}

0 commit comments

Comments
 (0)