Skip to content

Commit 9a0640f

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. sm_util_htoa and sm_util_atoh replaced with bin2hex and hex2bin. Signed-off-by: Markus Lassila <markus.lassila@nordicsemi.no>
1 parent 2fbe55d commit 9a0640f

4 files changed

Lines changed: 58 additions & 152 deletions

File tree

‎app/src/lwm2m_carrier/sm_at_carrier.c‎

Lines changed: 35 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ static void print_deferred(const lwm2m_carrier_event_t *evt)
9797

9898
static void on_event_app_data(const lwm2m_carrier_event_t *event)
9999
{
100-
int ret;
100+
size_t size;
101101
lwm2m_carrier_event_app_data_t *app_data = event->data.app_data;
102102

103103
if (app_data->path_len > ARRAY_SIZE(app_data->path)) {
@@ -122,16 +122,16 @@ 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));
127-
if (ret < 0) {
128-
LOG_ERR("Failed to encode hex array to hex string: %d", ret);
125+
size = bin2hex(app_data->buffer, app_data->buffer_len, sm_data_buf,
126+
sizeof(sm_data_buf));
127+
if (size == 0) {
128+
LOG_ERR("Failed to encode array to hex string");
129129
return;
130130
}
131131

132-
rsp_send("\r\n#XCARRIEREVT: %u,%hhu,\"%s\",%d\r\n\"", event->type, app_data->type,
133-
uri_path, ret);
134-
data_send(sm_data_buf, ret);
132+
rsp_send("\r\n#XCARRIEREVT: %u,%hhu,\"%s\",%zu\r\n\"", event->type, app_data->type,
133+
uri_path, size);
134+
data_send(sm_data_buf, size);
135135
rsp_send("\"");
136136
} else {
137137
rsp_send("\r\n#XCARRIEREVT: %u,%hhu,\"%s\"\r\n", event->type, app_data->type,
@@ -261,16 +261,16 @@ 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);
265-
if (ret < 0) {
266-
LOG_ERR("Failed to decode hex string to hex array");
267-
return ret;
264+
size = hex2bin(data, len, sm_data_buf, size);
265+
if (size == 0) {
266+
LOG_ERR("Failed to decode hex string to array");
267+
return -EINVAL;
268268
}
269269

270270
uint16_t path[3] = { LWM2M_CARRIER_OBJECT_APP_DATA_CONTAINER, 0, 0 };
271271
uint8_t path_len = 3;
272272

273-
ret = lwm2m_carrier_app_data_set(path, path_len, sm_data_buf, ret);
273+
ret = lwm2m_carrier_app_data_set(path, path_len, sm_data_buf, size);
274274
LOG_INF("datamode send: %d", ret);
275275
if (ret < 0) {
276276
exit_datamode_handler(ret);
@@ -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+
uint8_t 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,16 +343,16 @@ 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);
347-
if (ret < 0) {
348-
LOG_ERR("Failed to decode hex string to hex array");
349-
return ret;
346+
data_len = hex2bin(data_ascii, data_ascii_len, data, data_len);
347+
if (data_len == 0) {
348+
LOG_ERR("Failed to decode hex string to array");
349+
return -EINVAL;
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, data_len);
353353
} else if (param_count == 4 || param_count == 5) {
354-
uint8_t *data = NULL;
355-
int size = 0;
354+
uint8_t *data_ptr = NULL;
355+
size_t size = 0;
356356

357357
uint16_t inst_id;
358358
uint16_t res_inst_id;
@@ -378,17 +378,16 @@ 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);
382-
if (ret < 0) {
383-
LOG_ERR("Failed to decode hex string to hex array");
384-
return ret;
381+
size = hex2bin(data_ascii, data_ascii_len, data, data_len);
382+
if (size == 0) {
383+
LOG_ERR("Failed to decode hex string to array");
384+
return -EINVAL;
385385
}
386386

387-
data = data_hex;
388-
size = ret;
387+
data_ptr = data;
389388
}
390389

391-
ret = lwm2m_carrier_app_data_set(path, path_len, data, size);
390+
ret = lwm2m_carrier_app_data_set(path, path_len, data_ptr, size);
392391
}
393392

394393
return ret;
@@ -746,21 +745,21 @@ static int do_carrier_event_log_log_data(enum at_parser_cmd_type, struct at_pars
746745
char data_ascii[CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN] = {0};
747746
size_t data_ascii_len = CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN;
748747

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;
748+
uint8_t data[CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2];
749+
size_t data_len = CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2;
751750

752751
int ret = util_string_get(parser, 2, data_ascii, &data_ascii_len);
753752
if (ret) {
754753
return ret;
755754
}
756755

757-
ret = sm_util_atoh(data_ascii, data_ascii_len, data_hex, data_hex_len);
758-
if (ret < 0) {
759-
LOG_ERR("Failed to decode hex string to hex array");
760-
return ret;
756+
data_len = hex2bin(data_ascii, data_ascii_len, data, data_len);
757+
if (data_len == 0) {
758+
LOG_ERR("Failed to decode hex string to array");
759+
return -EINVAL;
761760
}
762761

763-
return lwm2m_carrier_log_data_set(data_hex, ret);
762+
return lwm2m_carrier_log_data_set(data, data_len);
764763
}
765764

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

‎app/src/sm_at_socket.c‎

Lines changed: 23 additions & 23 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 uint8_t bin_data[1400]; /* Buffer for hex2bin data conversion */
9595

9696
static struct async_poll_ctx {
9797
struct k_work poll_work; /* Work to send poll URCs. */
@@ -1017,17 +1017,18 @@ static int do_send(struct sm_socket *sock, const uint8_t *data, int len, int fla
10171017

10181018
static int data_send_hex(struct sm_socket *sock, const uint8_t *buf, int recv_len)
10191019
{
1020-
int consumed = 0;
1021-
char hex_buf[256] = {0};
1022-
uint16_t data_len = recv_len < (sizeof(hex_buf) / 2) ? recv_len : (sizeof(hex_buf) / 2);
1020+
size_t consumed = 0;
1021+
char hex_buf[257] = {0};
1022+
uint16_t data_len =
1023+
recv_len < (sizeof(hex_buf) - 1) / 2 ? recv_len : (sizeof(hex_buf) - 1) / 2;
10231024

10241025
/* For hex string mode, convert the received data to hex string */
10251026
while (consumed < recv_len) {
1026-
int size = sm_util_htoa(buf + consumed, data_len, hex_buf, sizeof(hex_buf));
1027+
size_t size = bin2hex(buf + consumed, data_len, hex_buf, sizeof(hex_buf));
10271028

1028-
if (size < 0) {
1029+
if (size == 0) {
10291030
LOG_ERR("Failed to convert binary data to hex string");
1030-
return size;
1031+
return -EINVAL;
10311032
}
10321033
data_send(hex_buf, size);
10331034
consumed += size / 2; /* size is in hex string length */
@@ -1680,7 +1681,7 @@ STATIC int handle_at_send(enum at_parser_cmd_type cmd_type, struct at_parser *pa
16801681
int err = -EINVAL;
16811682
int fd;
16821683
uint16_t mode;
1683-
int size;
1684+
size_t size;
16841685
struct sm_socket *sock = NULL;
16851686
const char *str_ptr;
16861687
int data_len = 0;
@@ -1715,17 +1716,16 @@ STATIC int handle_at_send(enum at_parser_cmd_type cmd_type, struct at_parser *pa
17151716

17161717
/* Convert hex string to binary data */
17171718
if (mode == AT_SOCKET_MODE_HEX) {
1718-
err = sm_util_atoh(str_ptr, size, hex_data, sizeof(hex_data));
1719-
if (err < 0) {
1719+
size = hex2bin(str_ptr, size, bin_data, sizeof(bin_data));
1720+
if (size == 0) {
17201721
LOG_ERR("Failed to convert hex string to binary data");
1721-
return err;
1722+
return -EINVAL;
17221723
}
1723-
str_ptr = hex_data;
1724-
size = err;
1724+
str_ptr = (const char *)bin_data;
17251725
}
17261726

1727-
err = do_send(sock, str_ptr, size, sock->send_flags);
1728-
if (err == size) {
1727+
err = do_send(sock, (uint8_t *)str_ptr, (int)size, sock->send_flags);
1728+
if (err == (int)size) {
17291729
err = 0;
17301730
} else {
17311731
err = err < 0 ? err : -EAGAIN;
@@ -1819,7 +1819,7 @@ STATIC int handle_at_sendto(enum at_parser_cmd_type cmd_type, struct at_parser *
18191819
int err = -EINVAL;
18201820
int fd;
18211821
uint16_t mode;
1822-
int size;
1822+
size_t size;
18231823
struct sm_socket *sock = NULL;
18241824
const char *str_ptr;
18251825
int data_len = 0;
@@ -1863,17 +1863,17 @@ 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));
1867-
if (err < 0) {
1866+
size = hex2bin(str_ptr, size, bin_data, sizeof(bin_data));
1867+
if (size == 0) {
18681868
LOG_ERR("Failed to convert hex string to binary data");
1869-
return err;
1869+
return -EINVAL;
18701870
}
1871-
str_ptr = hex_data;
1872-
size = err;
1871+
str_ptr = (const char *)bin_data;
18731872
}
18741873

1875-
err = do_sendto(sock, udp_url, udp_port, str_ptr, size, sock->send_flags);
1876-
if (err == size) {
1874+
err = do_sendto(sock, udp_url, udp_port, (const uint8_t *)str_ptr,
1875+
(int)size, sock->send_flags);
1876+
if (err == (int)size) {
18771877
err = 0;
18781878
} else {
18791879
err = err < 0 ? err : -EAGAIN;

‎app/src/sm_util.c‎

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -182,63 +182,6 @@ 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)
186-
{
187-
for (int i = 0; i < data_len; i++) {
188-
char ch = *(data + i);
189-
190-
if ((ch < '0' || ch > '9') &&
191-
(ch < 'A' || ch > 'F') &&
192-
(ch < 'a' || ch > 'f')) {
193-
return false;
194-
}
195-
}
196-
197-
return true;
198-
}
199-
200-
int sm_util_htoa(const uint8_t *hex, uint16_t hex_len, char *ascii, uint16_t ascii_len)
201-
{
202-
if (hex == NULL || ascii == NULL) {
203-
return -EINVAL;
204-
}
205-
if (ascii_len < (hex_len * 2)) {
206-
return -EINVAL;
207-
}
208-
209-
for (int i = 0; i < hex_len; i++) {
210-
sprintf(ascii + (i * 2), "%02X", *(hex + i));
211-
}
212-
213-
return (hex_len * 2);
214-
}
215-
216-
int sm_util_atoh(const char *ascii, uint16_t ascii_len, uint8_t *hex, uint16_t hex_len)
217-
{
218-
char hex_str[3];
219-
220-
if (hex == NULL || ascii == NULL) {
221-
return -EINVAL;
222-
}
223-
if ((ascii_len % 2) > 0) {
224-
return -EINVAL;
225-
}
226-
if (ascii_len > (hex_len * 2)) {
227-
return -EINVAL;
228-
}
229-
if (!sm_util_hexstr_check(ascii, ascii_len)) {
230-
return -EINVAL;
231-
}
232-
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);
237-
}
238-
239-
return (ascii_len / 2);
240-
}
241-
242185
int util_string_get(struct at_parser *parser, size_t index, char *value, size_t *len)
243186
{
244187
int ret;

‎app/src/sm_util.h‎

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -60,42 +60,6 @@ int sm_util_at_cmd_no_intercept(char *buf, size_t len, const char *at_cmd);
6060
*/
6161
bool sm_util_casecmp(const char *str1, const char *str2);
6262

63-
/**
64-
* @brief Detect hexdecimal string data type
65-
*
66-
* @param[in] data Hexdecimal string arrary to be checked
67-
* @param[in] data_len Length of array
68-
*
69-
* @return true if the input is hexdecimal string array, otherwise false
70-
*/
71-
bool sm_util_hexstr_check(const uint8_t *data, uint16_t data_len);
72-
73-
/**
74-
* @brief Encode hex array to hexdecimal string (ASCII text)
75-
*
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
80-
*
81-
* @return actual size of ascii string if the operation was successful.
82-
* Otherwise, a (negative) error code is returned.
83-
*/
84-
int sm_util_htoa(const uint8_t *hex, uint16_t hex_len, char *ascii, uint16_t ascii_len);
85-
86-
/**
87-
* @brief Decode hexdecimal string (ASCII text) to hex array
88-
*
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
93-
*
94-
* @return actual size of hex array if the operation was successful.
95-
* Otherwise, a (negative) error code is returned.
96-
*/
97-
int sm_util_atoh(const char *ascii, uint16_t ascii_len, uint8_t *hex, uint16_t hex_len);
98-
9963
/**
10064
* @brief Get string value from AT command with length check.
10165
*

0 commit comments

Comments
 (0)