From 9e0d97d2e24b1d3999069c1ce5d699482f590c04 Mon Sep 17 00:00:00 2001 From: Xavier Del Campo Date: Sat, 11 Sep 2021 00:50:04 +0200 Subject: [PATCH 1/3] at.c: use snprintf(3) snprintf(3) is preferred over insecure sprintf(3) in order to avoid buffer overrun vulnerabilities. --- src/at.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/at.c b/src/at.c index ff77fbc..168f572 100644 --- a/src/at.c +++ b/src/at.c @@ -58,15 +58,21 @@ gboolean at_send_command(struct EG25Manager *manager) /* Send AT command */ if (at_cmd->subcmd == NULL && at_cmd->value == NULL && at_cmd->expected == NULL) - len = sprintf(command, "AT+%s\r\n", at_cmd->cmd); + len = snprintf(command, sizeof(command), "AT+%s\r\n", at_cmd->cmd); else if (at_cmd->subcmd == NULL && at_cmd->value == NULL) - len = sprintf(command, "AT+%s?\r\n", at_cmd->cmd); + len = snprintf(command, sizeof(command), "AT+%s?\r\n", at_cmd->cmd); else if (at_cmd->subcmd == NULL && at_cmd->value) - len = sprintf(command, "AT+%s=%s\r\n", at_cmd->cmd, at_cmd->value); + len = snprintf(command, sizeof(command),"AT+%s=%s\r\n", at_cmd->cmd, at_cmd->value); else if (at_cmd->subcmd && at_cmd->value == NULL) - len = sprintf(command, "AT+%s=\"%s\"\r\n", at_cmd->cmd, at_cmd->subcmd); + len = snprintf(command, sizeof(command), "AT+%s=\"%s\"\r\n", at_cmd->cmd, at_cmd->subcmd); else if (at_cmd->subcmd && at_cmd->value) - len = sprintf(command, "AT+%s=\"%s\",%s\r\n", at_cmd->cmd, at_cmd->subcmd, at_cmd->value); + len = snprintf(command, sizeof(command), "AT+%s=\"%s\",%s\r\n", at_cmd->cmd, at_cmd->subcmd, at_cmd->value); + + if (len < 0 || len >= sizeof(command)) { + g_warning("AT command does not fit into buffer\n"); + return FALSE; + } + manager->at_callback = at_cmd->callback; ret = write(manager->at_fd, command, len); From 771e9f8316e6795d4dacf1ebc333d137ff18a41c Mon Sep 17 00:00:00 2001 From: Xavier Del Campo Date: Tue, 28 Sep 2021 23:17:25 +0200 Subject: [PATCH 2/3] at_send_command: call at_next_command on failure --- src/at.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/at.c b/src/at.c index 168f572..399913c 100644 --- a/src/at.c +++ b/src/at.c @@ -70,6 +70,7 @@ gboolean at_send_command(struct EG25Manager *manager) if (len < 0 || len >= sizeof(command)) { g_warning("AT command does not fit into buffer\n"); + at_next_command(manager); return FALSE; } From 6f913894964b2df39ed426bf56ec94376a406597 Mon Sep 17 00:00:00 2001 From: Xavier Del Campo Date: Tue, 28 Sep 2021 23:25:19 +0200 Subject: [PATCH 3/3] at_send_command: improve logging when snprintf(3) fails --- src/at.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/at.c b/src/at.c index 399913c..2527bca 100644 --- a/src/at.c +++ b/src/at.c @@ -68,8 +68,14 @@ gboolean at_send_command(struct EG25Manager *manager) else if (at_cmd->subcmd && at_cmd->value) len = snprintf(command, sizeof(command), "AT+%s=\"%s\",%s\r\n", at_cmd->cmd, at_cmd->subcmd, at_cmd->value); - if (len < 0 || len >= sizeof(command)) { - g_warning("AT command does not fit into buffer\n"); + if (len < 0) { + g_warning("snprintf(3) failed"); + at_next_command(manager); + return FALSE; + } + else if (len >= sizeof(command)) { + g_warning("AT command does not fit into buffer " + "(%d bytes required, %zu available)", len, sizeof(command)); at_next_command(manager); return FALSE; }