Skip to content

Commit 54c5cc9

Browse files
rzrgithub-advanced-security[bot]
rzr
and
github-advanced-security[bot]
committed
controller: Harden zwave_controller_utils.c 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. 13,14: 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 49326bd commit 54c5cc9

File tree

1 file changed

+151
-57
lines changed

1 file changed

+151
-57
lines changed

‎applications/zpc/components/zwave/zwave_controller/src/zwave_controller_utils.c

Lines changed: 151 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*****************************************************************************/
1313

1414
//Generic includes
15+
#include <assert.h>
1516
#include <stdlib.h>
1617
#include <stdio.h>
1718

@@ -76,37 +77,73 @@ void zwave_sl_log_frame_data(
7677
char message[DEBUG_MESSAGE_BUFFER_LENGTH];
7778
uint16_t index = 0;
7879

79-
index += snprintf(message + index,
80-
sizeof(message) - index,
81-
"Z-Wave Frame (NodeID %d:%d -> %d:%d ",
82-
connection_info->remote.node_id,
83-
connection_info->remote.endpoint_id,
84-
connection_info->local.node_id,
85-
connection_info->local.endpoint_id);
80+
int written = snprintf(message + index,
81+
sizeof(message) - index,
82+
"Z-Wave Frame (NodeID %d:%d -> %d:%d ",
83+
connection_info->remote.node_id,
84+
connection_info->remote.endpoint_id,
85+
connection_info->local.node_id,
86+
connection_info->local.endpoint_id);
87+
if (written < 0 || written >= (int)(sizeof(message) - index)) {
88+
assert(false);
89+
sl_log_error(LOG_TAG, "Overflow in zwave_sl_log_frame_data\n");
90+
return;
91+
}
92+
index += written;
8693

8794
if (connection_info->local.is_multicast) {
88-
index += snprintf(message + index,
89-
sizeof(message) - index,
90-
"via multicast/broadcast.");
95+
written = snprintf(message + index,
96+
sizeof(message) - index,
97+
"via multicast/broadcast.");
98+
if (written < 0 || written >= (int)(sizeof(message) - index)) {
99+
assert(false);
100+
sl_log_error(LOG_TAG, "Overflow in zwave_sl_log_frame_data\n");
101+
return;
102+
}
103+
index += written;
91104
} else {
92-
index
93-
+= snprintf(message + index, sizeof(message) - index, "via singlecast.");
105+
written
106+
= snprintf(message + index, sizeof(message) - index, "via singlecast.");
107+
if (written < 0 || written >= (int)(sizeof(message) - index)) {
108+
assert(false);
109+
sl_log_error(LOG_TAG, "Overflow in zwave_sl_log_frame_data\n");
110+
return;
111+
}
112+
index += written;
94113
}
95114

96-
index += snprintf(message + index,
97-
sizeof(message) - index,
98-
" Status flags: 0x%02X - ",
99-
rx_options->status_flags);
100-
index += snprintf(message + index,
101-
sizeof(message) - index,
102-
"RSSI: %d dBm - Frame payload (hex): ",
103-
rx_options->rssi);
115+
written = snprintf(message + index,
116+
sizeof(message) - index,
117+
" Status flags: 0x%02X - ",
118+
rx_options->status_flags);
119+
if (written < 0 || written >= (int)(sizeof(message) - index)) {
120+
assert(false);
121+
sl_log_error(LOG_TAG, "Overflow in zwave_sl_log_frame_data\n");
122+
return;
123+
}
124+
index += written;
125+
written = snprintf(message + index,
126+
sizeof(message) - index,
127+
"RSSI: %d dBm - Frame payload (hex): ",
128+
rx_options->rssi);
129+
if (written < 0 || written >= (int)(sizeof(message) - index)) {
130+
assert(false);
131+
sl_log_error(LOG_TAG, "Overflow in zwave_sl_log_frame_data\n");
132+
return;
133+
}
134+
index += written;
104135

105136
for (uint8_t i = 0; i < frame_length; i++) {
106-
index += snprintf(message + index,
107-
sizeof(message) - index,
108-
"%02X ",
109-
frame_data[i]);
137+
written = snprintf(message + index,
138+
sizeof(message) - index,
139+
"%02X ",
140+
frame_data[i]);
141+
if (written < 0 || written >= (int)(sizeof(message) - index)) {
142+
assert(false);
143+
sl_log_error(LOG_TAG, "Overflow in zwave_sl_log_frame_data\n");
144+
return;
145+
}
146+
index += written;
110147
}
111148
sl_log_debug(LOG_TAG, "%s\n", message);
112149
}
@@ -118,22 +155,41 @@ void zwave_sl_log_nif_data(zwave_node_id_t node_id,
118155
char message[DEBUG_MESSAGE_BUFFER_LENGTH];
119156
uint16_t index = 0;
120157

121-
index += snprintf(message + index,
122-
sizeof(message) - index,
123-
"NIF from NodeID: %d",
124-
node_id);
125-
126-
index += snprintf(message + index,
127-
sizeof(message) - index,
128-
" Capability/Security bytes: 0x%02X 0x%02X - ",
129-
node_info->listening_protocol,
130-
node_info->optional_protocol);
158+
int written = snprintf(message + index,
159+
sizeof(message) - index,
160+
"NIF from NodeID: %d",
161+
node_id);
162+
if (written < 0 || written >= (int)(sizeof(message) - index)) {
163+
sl_log_error(LOG_TAG, "Overflow in zwave_sl_log_nif_data\n");
164+
assert(false);
165+
return;
166+
}
167+
index += written;
168+
169+
written = snprintf(message + index,
170+
sizeof(message) - index,
171+
" Capability/Security bytes: 0x%02X 0x%02X - ",
172+
node_info->listening_protocol,
173+
node_info->optional_protocol);
174+
if (written < 0 || written >= (int)(sizeof(message) - index)) {
175+
sl_log_error(LOG_TAG, "Overflow in zwave_sl_log_nif_data\n");
176+
assert(false);
177+
return;
178+
}
179+
index += written;
131180

132181
if (node_info->optional_protocol
133182
& ZWAVE_NODE_INFO_OPTIONAL_PROTOCOL_CONTROLLER_MASK) {
134-
index += snprintf(message + index,
135-
sizeof(message) - index,
136-
"The node is a controller - ");
183+
written = snprintf(message + index,
184+
sizeof(message) - index,
185+
"The node is a controller - ");
186+
if (written < 0 || written >= (int)(sizeof(message) - index)) {
187+
sl_log_error(LOG_TAG, "Overflow in zwave_sl_log_nif_data\n");
188+
assert(false);
189+
return;
190+
}
191+
index += written;
192+
137193
} else {
138194
index += snprintf(message + index,
139195
sizeof(message) - index,
@@ -142,32 +198,64 @@ void zwave_sl_log_nif_data(zwave_node_id_t node_id,
142198

143199
if (node_info->listening_protocol
144200
& ZWAVE_NODE_INFO_LISTENING_PROTOCOL_LISTENING_MASK) {
145-
index += snprintf(message + index, sizeof(message) - index, "AL mode - ");
201+
written = snprintf(message + index, sizeof(message) - index, "AL mode - ");
202+
if (written < 0 || written >= (int)(sizeof(message) - index)) {
203+
sl_log_error(LOG_TAG, "Overflow in zwave_sl_log_nif_data\n");
204+
assert(false);
205+
return;
206+
}
207+
index += written;
208+
146209
} else if (node_info->optional_protocol
147210
& (ZWAVE_NODE_INFO_OPTIONAL_PROTOCOL_SENSOR_1000MS_MASK
148211
| ZWAVE_NODE_INFO_OPTIONAL_PROTOCOL_SENSOR_250MS_MASK)) {
149-
index += snprintf(message + index,
150-
sizeof(message) - index,
151-
"FL mode (FLiRS) - ");
212+
written = snprintf(message + index,
213+
sizeof(message) - index,
214+
"FL mode (FLiRS) - ");
215+
if (written < 0 || written >= (int)(sizeof(message) - index)) {
216+
sl_log_error(LOG_TAG, "Overflow in zwave_sl_log_nif_data\n");
217+
assert(false);
218+
return;
219+
}
220+
index += written;
221+
152222
} else {
153-
index += snprintf(message + index,
154-
sizeof(message) - index,
155-
"NL mode (Non-Listening) - ");
223+
written = snprintf(message + index,
224+
sizeof(message) - index,
225+
"NL mode (Non-Listening) - ");
226+
if (written < 0 || written >= (int)(sizeof(message) - index)) {
227+
sl_log_error(LOG_TAG, "Overflow in zwave_sl_log_nif_data\n");
228+
assert(false);
229+
return;
230+
}
231+
index += written;
156232
}
157233

158-
index += snprintf(message + index,
159-
sizeof(message) - index,
160-
"Basic, Generic and Specific Device Classes: 0x%02X 0x%02X "
161-
"0x%02X - Supported CC list: ",
162-
node_info->basic_device_class,
163-
node_info->generic_device_class,
164-
node_info->specific_device_class);
165-
234+
written
235+
= snprintf(message + index,
236+
sizeof(message) - index,
237+
"Basic, Generic and Specific Device Classes: 0x%02X 0x%02X "
238+
"0x%02X - Supported CC list: ",
239+
node_info->basic_device_class,
240+
node_info->generic_device_class,
241+
node_info->specific_device_class);
242+
if (written < 0 || written >= (int)(sizeof(message) - index)) {
243+
sl_log_error(LOG_TAG, "Overflow in zwave_sl_log_nif_data\n");
244+
assert(false);
245+
return;
246+
}
247+
index += written;
166248
for (uint8_t i = 0; i < node_info->command_class_list_length; i++) {
167-
index += snprintf(message + index,
168-
sizeof(message) - index,
169-
"%02X ",
170-
node_info->command_class_list[i]);
249+
written = snprintf(message + index,
250+
sizeof(message) - index,
251+
"%02X ",
252+
node_info->command_class_list[i]);
253+
if (written < 0 || written >= (int)(sizeof(message) - index)) {
254+
sl_log_error(LOG_TAG, "Overflow in zwave_sl_log_nif_data\n");
255+
assert(false);
256+
return;
257+
}
258+
index += written;
171259
}
172260

173261
sl_log_debug(LOG_TAG, "%s", message);
@@ -178,7 +266,13 @@ void zwave_sl_log_dsk(const char *tag, const zwave_dsk_t dsk)
178266
(void)tag; // Unused in Z-Wave build.
179267
char message[DEBUG_MESSAGE_BUFFER_LENGTH];
180268
uint16_t index = 0;
181-
index += snprintf(message + index, sizeof(message) - index, "DSK: ");
269+
int written = snprintf(message + index, sizeof(message) - index, "DSK: ");
270+
if (written < 0 || written >= (int)(sizeof(message) - index)) {
271+
sl_log_error(LOG_TAG, "Overflow in zwave_sl_log_dsk\n");
272+
assert(false);
273+
return;
274+
}
275+
index += written;
182276
zpc_converters_dsk_to_str(dsk, message + index, sizeof(message) - index);
183277
sl_log_info(tag, "%s\n", message);
184278
}

0 commit comments

Comments
 (0)