Skip to content

app: Fix stack corruption with binary to hex conversion #173

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MarkusLassila merged 1 commit into from
Feb 10, 2026
Merged

app: Fix stack corruption with binary to hex conversion #173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 35 additions & 36 deletions app/src/lwm2m_carrier/sm_at_carrier.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ static void print_deferred(const lwm2m_carrier_event_t *evt)

static void on_event_app_data(const lwm2m_carrier_event_t *event)
{
int ret;
size_t size;
lwm2m_carrier_event_app_data_t *app_data = event->data.app_data;

if (app_data->path_len > ARRAY_SIZE(app_data->path)) {
Expand All @@ -122,16 +122,16 @@ static void on_event_app_data(const lwm2m_carrier_event_t *event)
* SM_CARRIER_APP_DATA_BUFFER_LEN to account for the hex string.
* However, sm_data_buf is not expected to receive more than 512 bytes in downlink.
*/
ret = sm_util_htoa(app_data->buffer, app_data->buffer_len,
sm_data_buf, sizeof(sm_data_buf));
if (ret < 0) {
LOG_ERR("Failed to encode hex array to hex string: %d", ret);
size = bin2hex(app_data->buffer, app_data->buffer_len, sm_data_buf,
sizeof(sm_data_buf));
if (size == 0) {
LOG_ERR("Failed to encode array to hex string");
return;
}

rsp_send("\r\n#XCARRIEREVT: %u,%hhu,\"%s\",%d\r\n\"", event->type, app_data->type,
uri_path, ret);
data_send(sm_data_buf, ret);
rsp_send("\r\n#XCARRIEREVT: %u,%hhu,\"%s\",%zu\r\n\"", event->type, app_data->type,
uri_path, size);
data_send(sm_data_buf, size);
rsp_send("\"");
} else {
rsp_send("\r\n#XCARRIEREVT: %u,%hhu,\"%s\"\r\n", event->type, app_data->type,
Expand Down Expand Up @@ -261,16 +261,16 @@ static int carrier_datamode_callback(uint8_t op, const uint8_t *data, int len, u

size_t size = CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2;

ret = sm_util_atoh(data, len, sm_data_buf, size);
if (ret < 0) {
LOG_ERR("Failed to decode hex string to hex array");
return ret;
size = hex2bin(data, len, sm_data_buf, size);
if (size == 0) {
LOG_ERR("Failed to decode hex string to array");
return -EINVAL;
}

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

ret = lwm2m_carrier_app_data_set(path, path_len, sm_data_buf, ret);
ret = lwm2m_carrier_app_data_set(path, path_len, sm_data_buf, size);
LOG_INF("datamode send: %d", ret);
if (ret < 0) {
exit_datamode_handler(ret);
Expand Down Expand Up @@ -331,8 +331,8 @@ static int do_carrier_appdata_set(enum at_parser_cmd_type, struct at_parser *par
char data_ascii[CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN] = {0};
size_t data_ascii_len = CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN;

char data_hex[CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2];
size_t data_hex_len = CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2;
uint8_t data[CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2];
size_t data_len = CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2;

if (param_count == 3) {
uint16_t path[3] = { LWM2M_CARRIER_OBJECT_APP_DATA_CONTAINER, 0, 0 };
Expand All @@ -343,16 +343,16 @@ static int do_carrier_appdata_set(enum at_parser_cmd_type, struct at_parser *par
return ret;
}

ret = sm_util_atoh(data_ascii, data_ascii_len, data_hex, data_hex_len);
if (ret < 0) {
LOG_ERR("Failed to decode hex string to hex array");
return ret;
data_len = hex2bin(data_ascii, data_ascii_len, data, data_len);
if (data_len == 0) {
LOG_ERR("Failed to decode hex string to array");
return -EINVAL;
}

ret = lwm2m_carrier_app_data_set(path, path_len, data_hex, ret);
ret = lwm2m_carrier_app_data_set(path, path_len, data, data_len);
} else if (param_count == 4 || param_count == 5) {
uint8_t *data = NULL;
int size = 0;
uint8_t *data_ptr = NULL;
size_t size = 0;

uint16_t inst_id;
uint16_t res_inst_id;
Expand All @@ -378,17 +378,16 @@ static int do_carrier_appdata_set(enum at_parser_cmd_type, struct at_parser *par
return ret;
}

ret = sm_util_atoh(data_ascii, data_ascii_len, data_hex, data_hex_len);
if (ret < 0) {
LOG_ERR("Failed to decode hex string to hex array");
return ret;
size = hex2bin(data_ascii, data_ascii_len, data, data_len);
if (size == 0) {
LOG_ERR("Failed to decode hex string to array");
return -EINVAL;
}

data = data_hex;
size = ret;
data_ptr = data;
}

ret = lwm2m_carrier_app_data_set(path, path_len, data, size);
ret = lwm2m_carrier_app_data_set(path, path_len, data_ptr, size);
}

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

char data_hex[CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2];
size_t data_hex_len = CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2;
uint8_t data[CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2];
size_t data_len = CONFIG_SM_CARRIER_APP_DATA_BUFFER_LEN / 2;

int ret = util_string_get(parser, 2, data_ascii, &data_ascii_len);
if (ret) {
return ret;
}

ret = sm_util_atoh(data_ascii, data_ascii_len, data_hex, data_hex_len);
if (ret < 0) {
LOG_ERR("Failed to decode hex string to hex array");
return ret;
data_len = hex2bin(data_ascii, data_ascii_len, data, data_len);
if (data_len == 0) {
LOG_ERR("Failed to decode hex string to array");
return -EINVAL;
}

return lwm2m_carrier_log_data_set(data_hex, ret);
return lwm2m_carrier_log_data_set(data, data_len);
}

/* AT#XCARRIER="position",<latitude>,<longitude>,<altitude>,<timestamp>,<uncertainty> */
Expand Down
46 changes: 23 additions & 23 deletions app/src/sm_at_socket.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ static struct sm_socket {
} socks[SM_MAX_SOCKET_COUNT];

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

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

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

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

if (size < 0) {
if (size == 0) {
LOG_ERR("Failed to convert binary data to hex string");
return size;
return -EINVAL;
}
data_send(hex_buf, size);
consumed += size / 2; /* size is in hex string length */
Comment thread
MarkusLassila marked this conversation as resolved.
Expand Down Expand Up @@ -1680,7 +1681,7 @@ STATIC int handle_at_send(enum at_parser_cmd_type cmd_type, struct at_parser *pa
int err = -EINVAL;
int fd;
uint16_t mode;
int size;
size_t size;

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do_send() (per context) takes const uint8_t *data and an int len, but this code passes a const char * (str_ptr) and a size_t (size). This can trigger incompatible-pointer and narrowing conversion warnings and risks truncation if sizes ever exceed INT_MAX. Prefer keeping a const uint8_t *data_ptr for the payload buffer (cast once where needed), and either change do_send()/do_sendto() to accept size_t lengths or add an explicit, bounded cast at the call site.

Suggested change
size_t size;
int size;

Copilot uses AI. Check for mistakes.

@MarkusLassila MarkusLassila Feb 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

size to be dealt in later PR.

struct sm_socket *sock = NULL;
const char *str_ptr;
int data_len = 0;
Expand Down Expand Up @@ -1715,17 +1716,16 @@ STATIC int handle_at_send(enum at_parser_cmd_type cmd_type, struct at_parser *pa

/* Convert hex string to binary data */
if (mode == AT_SOCKET_MODE_HEX) {
err = sm_util_atoh(str_ptr, size, hex_data, sizeof(hex_data));
if (err < 0) {
size = hex2bin(str_ptr, size, bin_data, sizeof(bin_data));
Comment thread
trantanen marked this conversation as resolved.
if (size == 0) {
LOG_ERR("Failed to convert hex string to binary data");
return err;
return -EINVAL;
}
str_ptr = hex_data;
size = err;
str_ptr = (const char *)bin_data;
}

err = do_send(sock, str_ptr, size, sock->send_flags);
if (err == size) {
err = do_send(sock, (uint8_t *)str_ptr, (int)size, sock->send_flags);
if (err == (int)size) {
err = 0;
} else {
err = err < 0 ? err : -EAGAIN;
Expand Down Expand Up @@ -1819,7 +1819,7 @@ STATIC int handle_at_sendto(enum at_parser_cmd_type cmd_type, struct at_parser *
int err = -EINVAL;
int fd;
uint16_t mode;
int size;
size_t size;
struct sm_socket *sock = NULL;
const char *str_ptr;
int data_len = 0;
Expand Down Expand Up @@ -1863,17 +1863,17 @@ STATIC int handle_at_sendto(enum at_parser_cmd_type cmd_type, struct at_parser *

/* Convert hex string to binary data */
if (mode == AT_SOCKET_MODE_HEX) {
err = sm_util_atoh(str_ptr, size, hex_data, sizeof(hex_data));
if (err < 0) {
size = hex2bin(str_ptr, size, bin_data, sizeof(bin_data));
if (size == 0) {
LOG_ERR("Failed to convert hex string to binary data");
return err;
return -EINVAL;
}
str_ptr = hex_data;
size = err;
str_ptr = (const char *)bin_data;
}

err = do_sendto(sock, udp_url, udp_port, str_ptr, size, sock->send_flags);
if (err == size) {
err = do_sendto(sock, udp_url, udp_port, (const uint8_t *)str_ptr,
(int)size, sock->send_flags);
if (err == (int)size) {
err = 0;
} else {
err = err < 0 ? err : -EAGAIN;
Expand Down
57 changes: 0 additions & 57 deletions app/src/sm_util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,63 +182,6 @@ bool sm_util_casecmp(const char *str1, const char *str2)
return true;
}

bool sm_util_hexstr_check(const uint8_t *data, uint16_t data_len)
{
for (int i = 0; i < data_len; i++) {
char ch = *(data + i);

if ((ch < '0' || ch > '9') &&
(ch < 'A' || ch > 'F') &&
(ch < 'a' || ch > 'f')) {
return false;
}
}

return true;
}

int sm_util_htoa(const uint8_t *hex, uint16_t hex_len, char *ascii, uint16_t ascii_len)
{
if (hex == NULL || ascii == NULL) {
return -EINVAL;
}
if (ascii_len < (hex_len * 2)) {
return -EINVAL;
}

for (int i = 0; i < hex_len; i++) {
sprintf(ascii + (i * 2), "%02X", *(hex + i));

@MarkusLassila MarkusLassila Feb 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This caused the stack corruption. When filling the whole buffer, the terminating null-character was written beyond the buffer.

}

return (hex_len * 2);
}

int sm_util_atoh(const char *ascii, uint16_t ascii_len, uint8_t *hex, uint16_t hex_len)
{
char hex_str[3];

if (hex == NULL || ascii == NULL) {
return -EINVAL;
}
if ((ascii_len % 2) > 0) {
return -EINVAL;
}
if (ascii_len > (hex_len * 2)) {
return -EINVAL;
}
if (!sm_util_hexstr_check(ascii, ascii_len)) {
return -EINVAL;
}

hex_str[2] = '\0';
for (int i = 0; (i * 2) < ascii_len; i++) {
strncpy(&hex_str[0], ascii + (i * 2), 2);
*(hex + i) = (uint8_t)strtoul(hex_str, NULL, 16);
}

return (ascii_len / 2);
}

int util_string_get(struct at_parser *parser, size_t index, char *value, size_t *len)
{
int ret;
Expand Down
36 changes: 0 additions & 36 deletions app/src/sm_util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,42 +60,6 @@ int sm_util_at_cmd_no_intercept(char *buf, size_t len, const char *at_cmd);
*/
bool sm_util_casecmp(const char *str1, const char *str2);

/**
* @brief Detect hexdecimal string data type
*
* @param[in] data Hexdecimal string arrary to be checked
* @param[in] data_len Length of array
*
* @return true if the input is hexdecimal string array, otherwise false
*/
bool sm_util_hexstr_check(const uint8_t *data, uint16_t data_len);

/**
* @brief Encode hex array to hexdecimal string (ASCII text)
*
* @param[in] hex Hex arrary to be encoded
* @param[in] hex_len Length of hex array
* @param[out] ascii encoded hexdecimal string
* @param[in] ascii_len reserved buffer size
*
* @return actual size of ascii string if the operation was successful.
* Otherwise, a (negative) error code is returned.
*/
int sm_util_htoa(const uint8_t *hex, uint16_t hex_len, char *ascii, uint16_t ascii_len);

/**
* @brief Decode hexdecimal string (ASCII text) to hex array
*
* @param[in] ascii encoded hexdecimal string
* @param[in] ascii_len size of hexdecimal string
* @param[out] hex decoded hex arrary
* @param[in] hex_len reserved size of hex array
*
* @return actual size of hex array if the operation was successful.
* Otherwise, a (negative) error code is returned.
*/
int sm_util_atoh(const char *ascii, uint16_t ascii_len, uint8_t *hex, uint16_t hex_len);

/**
* @brief Get string value from AT command with length check.
*
Expand Down
6 changes: 3 additions & 3 deletions app/tests/at_socket/src/test_at_socket.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1422,8 +1422,8 @@ void test_xrecv_hex_string(void)
response = get_captured_response();
TEST_ASSERT_TRUE(strstr(response, "#XRECV:") != NULL);
TEST_ASSERT_TRUE(strstr(response, "1,1,5") != NULL); /* handle=1, mode=1, received=5 */
TEST_ASSERT_TRUE(strstr(response, "48656C6C6F") !=
NULL); /* Hex data in response (uppercase) */
TEST_ASSERT_TRUE(strstr(response, "48656c6c6f") !=
NULL); /* Hex data in response (lowercase) */
TEST_ASSERT_TRUE(strstr(response, "OK") != NULL);

/* Close socket */
Expand Down Expand Up @@ -1704,7 +1704,7 @@ void test_xrecvfrom_hex_string(void)
TEST_ASSERT_TRUE(strstr(response, "1,1,5") != NULL); /* handle=1, mode=1, received=5 */
TEST_ASSERT_TRUE(strstr(response, "10.0.0.1") != NULL); /* Source IP */
TEST_ASSERT_TRUE(strstr(response, "9000") != NULL); /* Source port */
TEST_ASSERT_TRUE(strstr(response, "48656C6C6F") != NULL); /* Hex data (uppercase) */
TEST_ASSERT_TRUE(strstr(response, "48656c6c6f") != NULL); /* Hex data (lowercase) */
TEST_ASSERT_TRUE(strstr(response, "OK") != NULL);

/* Close socket */
Expand Down