Skip to content

Commit b13b5bd

Browse files
MarkusLassilaMarkusLassila
committed
app: Fix stack corruption with binary to hex conversion
sm_util_htoa used sprintf to convert from binary to hex. With data sizes exceeding the buffer, the null-character was written beyond allocated space. In this particular case the socket pointer was corrupted. Reworked the bin2hex and hex2bin conversions. Signed-off-by: Markus Lassila <markus.lassila@nordicsemi.no>
1 parent 2fbe55d commit b13b5bd

4 files changed

Lines changed: 74 additions & 65 deletions

File tree

‎app/src/lwm2m_carrier/sm_at_carrier.c‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,10 @@ static void on_event_app_data(const lwm2m_carrier_event_t *event)
122122
* SM_CARRIER_APP_DATA_BUFFER_LEN to account for the hex string.
123123
* However, sm_data_buf is not expected to receive more than 512 bytes in downlink.
124124
*/
125-
ret = sm_util_htoa(app_data->buffer, app_data->buffer_len,
126-
sm_data_buf, sizeof(sm_data_buf));
125+
ret = sm_util_bin2hex(app_data->buffer, app_data->buffer_len, sm_data_buf,
126+
sizeof(sm_data_buf));
127127
if (ret < 0) {
128-
LOG_ERR("Failed to encode hex array to hex string: %d", ret);
128+
LOG_ERR("Failed to encode array to hex string: %d", ret);
129129
return;
130130
}
131131

@@ -261,9 +261,9 @@ static int carrier_datamode_callback(uint8_t op, const uint8_t *data, int len, u
261261

262262
size_t size = CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2;
263263

264-
ret = sm_util_atoh(data, len, sm_data_buf, size);
264+
ret = sm_util_hex2bin(data, len, sm_data_buf, size);
265265
if (ret < 0) {
266-
LOG_ERR("Failed to decode hex string to hex array");
266+
LOG_ERR("Failed to decode hex string to array");
267267
return ret;
268268
}
269269

@@ -331,8 +331,8 @@ static int do_carrier_appdata_set(enum at_parser_cmd_type, struct at_parser *par
331331
char data_ascii[CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN] = {0};
332332
size_t data_ascii_len = CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN;
333333

334-
char data_hex[CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2];
335-
size_t data_hex_len = CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2;
334+
char data[CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2];
335+
size_t data_len = CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2;
336336

337337
if (param_count == 3) {
338338
uint16_t path[3] = { LWM2M_CARRIER_OBJECT_APP_DATA_CONTAINER, 0, 0 };
@@ -343,15 +343,15 @@ static int do_carrier_appdata_set(enum at_parser_cmd_type, struct at_parser *par
343343
return ret;
344344
}
345345

346-
ret = sm_util_atoh(data_ascii, data_ascii_len, data_hex, data_hex_len);
346+
ret = sm_util_hex2bin(data_ascii, data_ascii_len, data, data_len);
347347
if (ret < 0) {
348-
LOG_ERR("Failed to decode hex string to hex array");
348+
LOG_ERR("Failed to decode hex string to array");
349349
return ret;
350350
}
351351

352-
ret = lwm2m_carrier_app_data_set(path, path_len, data_hex, ret);
352+
ret = lwm2m_carrier_app_data_set(path, path_len, data, ret);
353353
} else if (param_count == 4 || param_count == 5) {
354-
uint8_t *data = NULL;
354+
uint8_t *data_ptr = NULL;
355355
int size = 0;
356356

357357
uint16_t inst_id;
@@ -378,17 +378,17 @@ static int do_carrier_appdata_set(enum at_parser_cmd_type, struct at_parser *par
378378
return ret;
379379
}
380380

381-
ret = sm_util_atoh(data_ascii, data_ascii_len, data_hex, data_hex_len);
381+
ret = sm_util_hex2bin(data_ascii, data_ascii_len, data, data_len);
382382
if (ret < 0) {
383-
LOG_ERR("Failed to decode hex string to hex array");
383+
LOG_ERR("Failed to decode hex string to array");
384384
return ret;
385385
}
386386

387-
data = data_hex;
387+
data_ptr = data;
388388
size = ret;
389389
}
390390

391-
ret = lwm2m_carrier_app_data_set(path, path_len, data, size);
391+
ret = lwm2m_carrier_app_data_set(path, path_len, data_ptr, size);
392392
}
393393

394394
return ret;
@@ -746,21 +746,21 @@ static int do_carrier_event_log_log_data(enum at_parser_cmd_type, struct at_pars
746746
char data_ascii[CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN] = {0};
747747
size_t data_ascii_len = CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN;
748748

749-
char data_hex[CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2];
750-
size_t data_hex_len = CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2;
749+
char data[CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2];
750+
size_t data_len = CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2;
751751

752752
int ret = util_string_get(parser, 2, data_ascii, &data_ascii_len);
753753
if (ret) {
754754
return ret;
755755
}
756756

757-
ret = sm_util_atoh(data_ascii, data_ascii_len, data_hex, data_hex_len);
757+
ret = sm_util_hex2bin(data_ascii, data_ascii_len, data, data_len);
758758
if (ret < 0) {
759-
LOG_ERR("Failed to decode hex string to hex array");
759+
LOG_ERR("Failed to decode hex string to array");
760760
return ret;
761761
}
762762

763-
return lwm2m_carrier_log_data_set(data_hex, ret);
763+
return lwm2m_carrier_log_data_set(data, ret);
764764
}
765765

766766
/* AT#XCARRIER="position",<latitude>,<longitude>,<altitude>,<timestamp>,<uncertainty> */

‎app/src/sm_at_socket.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ static struct sm_socket {
9191
} socks[SM_MAX_SOCKET_COUNT];
9292

9393
static struct sm_socket *datamode_sock; /* Socket for data mode */
94-
static char hex_data[1400 + 1]; /* Buffer for hex data conversion */
94+
static char bin_data[1400 + 1]; /* Buffer for hex2bin data conversion */
9595

9696
static struct async_poll_ctx {
9797
struct k_work poll_work; /* Work to send poll URCs. */
@@ -1023,7 +1023,7 @@ static int data_send_hex(struct sm_socket *sock, const uint8_t *buf, int recv_le
10231023

10241024
/* For hex string mode, convert the received data to hex string */
10251025
while (consumed < recv_len) {
1026-
int size = sm_util_htoa(buf + consumed, data_len, hex_buf, sizeof(hex_buf));
1026+
int size = sm_util_bin2hex(buf + consumed, data_len, hex_buf, sizeof(hex_buf));
10271027

10281028
if (size < 0) {
10291029
LOG_ERR("Failed to convert binary data to hex string");
@@ -1715,12 +1715,12 @@ STATIC int handle_at_send(enum at_parser_cmd_type cmd_type, struct at_parser *pa
17151715

17161716
/* Convert hex string to binary data */
17171717
if (mode == AT_SOCKET_MODE_HEX) {
1718-
err = sm_util_atoh(str_ptr, size, hex_data, sizeof(hex_data));
1718+
err = sm_util_hex2bin(str_ptr, size, bin_data, sizeof(bin_data));
17191719
if (err < 0) {
17201720
LOG_ERR("Failed to convert hex string to binary data");
17211721
return err;
17221722
}
1723-
str_ptr = hex_data;
1723+
str_ptr = bin_data;
17241724
size = err;
17251725
}
17261726

@@ -1863,12 +1863,12 @@ STATIC int handle_at_sendto(enum at_parser_cmd_type cmd_type, struct at_parser *
18631863

18641864
/* Convert hex string to binary data */
18651865
if (mode == AT_SOCKET_MODE_HEX) {
1866-
err = sm_util_atoh(str_ptr, size, hex_data, sizeof(hex_data));
1866+
err = sm_util_hex2bin(str_ptr, size, bin_data, sizeof(bin_data));
18671867
if (err < 0) {
18681868
LOG_ERR("Failed to convert hex string to binary data");
18691869
return err;
18701870
}
1871-
str_ptr = hex_data;
1871+
str_ptr = bin_data;
18721872
size = err;
18731873
}
18741874

‎app/src/sm_util.c‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -182,58 +182,67 @@ bool sm_util_casecmp(const char *str1, const char *str2)
182182
return true;
183183
}
184184

185-
bool sm_util_hexstr_check(const uint8_t *data, uint16_t data_len)
185+
bool sm_util_hexstr_check(const uint8_t *ascii, uint16_t ascii_len)
186186
{
187-
for (int i = 0; i < data_len; i++) {
188-
char ch = *(data + i);
187+
for (int i = 0; i < ascii_len; i++) {
188+
char ch = ascii[i];
189189

190-
if ((ch < '0' || ch > '9') &&
191-
(ch < 'A' || ch > 'F') &&
192-
(ch < 'a' || ch > 'f')) {
190+
/* Check if character is valid hex: 0-9 or A-F (case-insensitive via & 0xDF) */
191+
if (!((ch >= '0' && ch <= '9') || ((ch & 0xDF) >= 'A' && (ch & 0xDF) <= 'F'))) {
193192
return false;
194193
}
195194
}
196195

197196
return true;
198197
}
199198

200-
int sm_util_htoa(const uint8_t *hex, uint16_t hex_len, char *ascii, uint16_t ascii_len)
199+
int sm_util_bin2hex(const uint8_t *data, uint16_t data_len, char *ascii, uint16_t ascii_len)
201200
{
202-
if (hex == NULL || ascii == NULL) {
201+
static const char hex_chars[] = "0123456789ABCDEF";
202+
203+
if (data == NULL || ascii == NULL) {
203204
return -EINVAL;
204205
}
205-
if (ascii_len < (hex_len * 2)) {
206+
if (ascii_len < (data_len * 2)) {
206207
return -EINVAL;
207208
}
208209

209-
for (int i = 0; i < hex_len; i++) {
210-
sprintf(ascii + (i * 2), "%02X", *(hex + i));
210+
for (int i = 0; i < data_len; i++) {
211+
uint8_t byte = data[i];
212+
213+
ascii[i * 2] = hex_chars[byte >> 4];
214+
ascii[i * 2 + 1] = hex_chars[byte & 0x0F];
211215
}
212216

213-
return (hex_len * 2);
217+
return (data_len * 2);
214218
}
215219

216-
int sm_util_atoh(const char *ascii, uint16_t ascii_len, uint8_t *hex, uint16_t hex_len)
220+
int sm_util_hex2bin(const char *ascii, uint16_t ascii_len, uint8_t *data, uint16_t data_len)
217221
{
218-
char hex_str[3];
219-
220-
if (hex == NULL || ascii == NULL) {
222+
if (data == NULL || ascii == NULL) {
221223
return -EINVAL;
222224
}
223225
if ((ascii_len % 2) > 0) {
224226
return -EINVAL;
225227
}
226-
if (ascii_len > (hex_len * 2)) {
228+
if (ascii_len > (data_len * 2)) {
227229
return -EINVAL;
228230
}
229231
if (!sm_util_hexstr_check(ascii, ascii_len)) {
230232
return -EINVAL;
231233
}
232234

233-
hex_str[2] = '\0';
234-
for (int i = 0; (i * 2) < ascii_len; i++) {
235-
strncpy(&hex_str[0], ascii + (i * 2), 2);
236-
*(hex + i) = (uint8_t)strtoul(hex_str, NULL, 16);
235+
for (int i = 0; i < ascii_len / 2; i++) {
236+
uint8_t high = ascii[i * 2];
237+
uint8_t low = ascii[i * 2 + 1];
238+
239+
/* Convert ASCII hex digit to value (0-15). The & 0xDF trick handles both
240+
* upper and lowercase by clearing bit 5, making 'a'-'f' equivalent to 'A'-'F'.
241+
*/
242+
high = (high <= '9') ? (high - '0') : ((high & 0xDF) - 'A' + 10);
243+
low = (low <= '9') ? (low - '0') : ((low & 0xDF) - 'A' + 10);
244+
245+
data[i] = (high << 4) | low;
237246
}
238247

239248
return (ascii_len / 2);

‎app/src/sm_util.h‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -61,40 +61,40 @@ int sm_util_at_cmd_no_intercept(char *buf, size_t len, const char *at_cmd);
6161
bool sm_util_casecmp(const char *str1, const char *str2);
6262

6363
/**
64-
* @brief Detect hexdecimal string data type
64+
* @brief Check if ASCII string contains valid hexadecimal characters
6565
*
66-
* @param[in] data Hexdecimal string arrary to be checked
67-
* @param[in] data_len Length of array
66+
* @param[in] ascii ASCII string to be validated (must contain only 0-9, A-F, a-f)
67+
* @param[in] ascii_len Length of string
6868
*
69-
* @return true if the input is hexdecimal string array, otherwise false
69+
* @return true if the input is a valid hexadecimal string, otherwise false
7070
*/
71-
bool sm_util_hexstr_check(const uint8_t *data, uint16_t data_len);
71+
bool sm_util_hexstr_check(const uint8_t *ascii, uint16_t ascii_len);
7272

7373
/**
74-
* @brief Encode hex array to hexdecimal string (ASCII text)
74+
* @brief Convert binary data to hexadecimal string (ASCII text)
7575
*
76-
* @param[in] hex Hex arrary to be encoded
77-
* @param[in] hex_len Length of hex array
78-
* @param[out] ascii encoded hexdecimal string
79-
* @param[in] ascii_len reserved buffer size
76+
* @param[in] data Binary data to be converted
77+
* @param[in] data_len Length of binary data
78+
* @param[out] ascii Output hexadecimal string (each byte becomes 2 hex chars)
79+
* @param[in] ascii_len Reserved buffer size (must be at least data_len * 2)
8080
*
8181
* @return actual size of ascii string if the operation was successful.
8282
* Otherwise, a (negative) error code is returned.
8383
*/
84-
int sm_util_htoa(const uint8_t *hex, uint16_t hex_len, char *ascii, uint16_t ascii_len);
84+
int sm_util_bin2hex(const uint8_t *data, uint16_t data_len, char *ascii, uint16_t ascii_len);
8585

8686
/**
87-
* @brief Decode hexdecimal string (ASCII text) to hex array
87+
* @brief Convert hexadecimal string (ASCII text) to binary data
8888
*
89-
* @param[in] ascii encoded hexdecimal string
90-
* @param[in] ascii_len size of hexdecimal string
91-
* @param[out] hex decoded hex arrary
92-
* @param[in] hex_len reserved size of hex array
89+
* @param[in] ascii Input hexadecimal string (2 hex chars per byte)
90+
* @param[in] ascii_len Size of hexadecimal string (must be even)
91+
* @param[out] data Output binary data
92+
* @param[in] data_len Reserved size of output buffer (must be at least ascii_len / 2)
9393
*
94-
* @return actual size of hex array if the operation was successful.
94+
* @return actual size of binary data if the operation was successful.
9595
* Otherwise, a (negative) error code is returned.
9696
*/
97-
int sm_util_atoh(const char *ascii, uint16_t ascii_len, uint8_t *hex, uint16_t hex_len);
97+
int sm_util_hex2bin(const char *ascii, uint16_t ascii_len, uint8_t *data, uint16_t data_len);
9898

9999
/**
100100
* @brief Get string value from AT command with length check.

0 commit comments

Comments
 (0)