Updated patch. Added \n to the debug messages by strlcpy and strlcat.
Paulius Zaleckas wrote:
This patch adds safe strcpy and strcat methods. Most of strcpy, strncpy,
strcat and strncat methods in gsmd was replaced by strlcpy and strlcat
methods. This also fixes couple potential bugs. I haven't noticed any
side effects, but additional testing would be very good.
Regards,
Paulius Zaleckas
Index: include/gsmd/gsmd.h
===================================================================
--- include/gsmd/gsmd.h (revision 4048)
+++ include/gsmd/gsmd.h (working copy)
@@ -12,6 +12,7 @@
#include <gsmd/vendorplugin.h>
#include <gsmd/select.h>
#include <gsmd/state.h>
+#include <gsmd/strl.h>
void *gsmd_tallocs;
Index: include/gsmd/strl.h
===================================================================
--- include/gsmd/strl.h (revision 0)
+++ include/gsmd/strl.h (revision 0)
@@ -0,0 +1,12 @@
+#ifndef __GSMD_STRL_H
+#define __GSMD_STRL_H
+
+#ifdef __GSMD__
+
+/* safe strcpy and strcat versions */
+extern size_t strlcpy(char *dest, const char *src, size_t size);
+extern size_t strlcat(char *dest, const char *src, size_t count);
+
+#endif /* __GSMD__ */
+
+#endif
Index: include/gsmd/extrsp.h
===================================================================
--- include/gsmd/extrsp.h (revision 4048)
+++ include/gsmd/extrsp.h (working copy)
@@ -7,6 +7,8 @@
/* how many individual sub-ranges can one range contain */
#define GSM_EXTRSP_MAX_RANGES 16
+/* how many character we are going to store in string buffer */
+#define GSM_EXTRSP_MAX_STRBUF 64
struct gsm_extrsp_range_item {
int min;
@@ -28,7 +30,7 @@
struct gsm_extrsp_range_item item[GSM_EXTRSP_MAX_RANGES];
int num_items;
} range;
- char string[64];
+ char string[GSM_EXTRSP_MAX_STRBUF];
int numeric;
} u;
};
Index: src/gsmd/ext_response.c
===================================================================
--- src/gsmd/ext_response.c (revision 4048)
+++ src/gsmd/ext_response.c (working copy)
@@ -122,12 +122,8 @@
break;
case TOKEN_STRING:
if (*cur == '"') {
- int len = strlen(buf);
- if (len > sizeof(cur_token->u.string)-1)
- len = sizeof(cur_token->u.string)-1;
-
/* end of string token */
- strncpy(cur_token->u.string, buf, len);
+ strlcpy(cur_token->u.string, buf, GSM_EXTRSP_MAX_STRBUF);
er->num_tokens++;
state = TOKEN_STRING_LASTQUOTE;
} else {
Index: src/gsmd/operator_cache.c
===================================================================
--- src/gsmd/operator_cache.c (revision 4048)
+++ src/gsmd/operator_cache.c (working copy)
@@ -82,7 +82,7 @@
strncpy(mcc, numeric_bcd_string, 3);
strncpy(mnc, numeric_bcd_string+3, 2);
- strncpy(op->alnum_long, alnum_long, sizeof(op->alnum_long-1));
+ strlcpy(op->alnum_long, alnum_long, sizeof(op->alnum_long));
op->numeric.mcc = atoi(mcc);
op->numeric.mnc = atoi(mnc);
Index: src/gsmd/usock.c
===================================================================
--- src/gsmd/usock.c (revision 4048)
+++ src/gsmd/usock.c (working copy)
@@ -160,7 +160,7 @@
gcs.stat = er->tokens[2].u.numeric;
gcs.mode = er->tokens[3].u.numeric;
gcs.mpty = er->tokens[4].u.numeric;
- strcpy(gcs.number, er->tokens[5].u.string);
+ strlcpy(gcs.number, er->tokens[5].u.string, GSMD_ADDR_MAXLEN+1);
gcs.type = er->tokens[6].u.numeric;
}
else if ( er->num_tokens == 8 &&
@@ -186,9 +186,9 @@
gcs.stat = er->tokens[2].u.numeric;
gcs.mode = er->tokens[3].u.numeric;
gcs.mpty = er->tokens[4].u.numeric;
- strcpy(gcs.number, er->tokens[5].u.string);
+ strlcpy(gcs.number, er->tokens[5].u.string, GSMD_ADDR_MAXLEN+1);
gcs.type = er->tokens[6].u.numeric;
- strncpy(gcs.alpha, er->tokens[7].u.string, 8+1);
+ strlcpy(gcs.alpha, er->tokens[7].u.string, GSMD_ALPHA_MAXLEN+1);
}
else {
DEBUGP("Invalid Input : Parse error\n");
@@ -257,7 +257,7 @@
gcfs.status = er->tokens[0].u.numeric;
gcfs.classx = er->tokens[1].u.numeric;
- strcpy(gcfs.addr.number, er->tokens[2].u.string);
+ strlcpy(gcfs.addr.number, er->tokens[2].u.string, GSMD_ADDR_MAXLEN+1);
gcfs.addr.type = er->tokens[3].u.numeric;
}
else if ( er->num_tokens == 7 &&
@@ -271,7 +271,7 @@
gcfs.status = er->tokens[0].u.numeric;
gcfs.classx = er->tokens[1].u.numeric;
- strcpy(gcfs.addr.number, er->tokens[2].u.string);
+ strlcpy(gcfs.addr.number, er->tokens[2].u.string, GSMD_ADDR_MAXLEN+1);
gcfs.addr.type = er->tokens[3].u.numeric;
gcfs.time = er->tokens[6].u.numeric;
}
@@ -560,18 +560,18 @@
if (!cmd)
return -ENOMEM;
- strncat(cmd->buf, gp->pin, sizeof(gp->pin));
+ strlcat(cmd->buf, gp->pin, cmd->buflen);
switch (gp->type) {
case GSMD_PIN_SIM_PUK:
case GSMD_PIN_SIM_PUK2:
- strcat(cmd->buf, "\",\"");
- strncat(cmd->buf, gp->newpin, sizeof(gp->newpin));
+ strlcat(cmd->buf, "\",\"", cmd->buflen);
+ strlcat(cmd->buf, gp->newpin, cmd->buflen);
break;
default:
break;
}
- strcat(cmd->buf, "\"");
+ strlcat(cmd->buf, "\"", cmd->buflen);
break;
case GSMD_PIN_GET_STATUS:
cmd = atcmd_fill("AT+CPIN?", 8 + 1, &get_cpin_cb, gu, 0, NULL);
@@ -718,7 +718,7 @@
er->tokens[1].type == GSMD_ECMD_RTT_STRING &&
er->tokens[2].type == GSMD_ECMD_RTT_NUMERIC) {
vmail.enable = er->tokens[0].u.numeric;
- strcpy(vmail.addr.number, er->tokens[1].u.string);
+ strlcpy(vmail.addr.number, er->tokens[1].u.string, GSMD_ADDR_MAXLEN+1);
vmail.addr.type = er->tokens[2].u.numeric;
}
rc = gsmd_ucmd_submit(gu, GSMD_MSG_NETWORK, GSMD_NETWORK_VMAIL_GET,
@@ -824,7 +824,7 @@
er->tokens[2].type == GSMD_ECMD_RTT_STRING ) {
- strcpy(buf, er->tokens[2].u.string);
+ strlcpy(buf, er->tokens[2].u.string, sizeof(buf));
}
else {
DEBUGP("Invalid Input : Parse error\n");
@@ -896,9 +896,12 @@
*/
out2->stat = er->tokens[0].u.numeric;
- strcpy(out2->opname_longalpha, er->tokens[1].u.string);
- strcpy(out2->opname_shortalpha, er->tokens[2].u.string);
- strcpy(out2->opname_num, er->tokens[3].u.string);
+ strlcpy(out2->opname_longalpha, er->tokens[1].u.string,
+ sizeof(out2->opname_longalpha));
+ strlcpy(out2->opname_shortalpha, er->tokens[2].u.string,
+ sizeof(out2->opname_shortalpha));
+ strlcpy(out2->opname_num, er->tokens[3].u.string,
+ sizeof(out2->opname_num));
}
else {
DEBUGP("Invalid Input : Parse error\n");
@@ -1131,9 +1134,9 @@
*/
gps.pb.index = er->tokens[0].u.numeric;
- strcpy(gps.pb.numb, er->tokens[1].u.string);
+ strlcpy(gps.pb.numb, er->tokens[1].u.string, GSMD_PB_NUMB_MAXLEN+1);
gps.pb.type = er->tokens[2].u.numeric;
- strcpy(gps.pb.text, er->tokens[3].u.string);
+ strlcpy(gps.pb.text, er->tokens[3].u.string, GSMD_PB_TEXT_MAXLEN+1);
}
else {
DEBUGP("Invalid Input : Parse error\n");
@@ -1180,9 +1183,9 @@
*/
gp.index = er->tokens[0].u.numeric;
- strcpy(gp.numb, er->tokens[1].u.string);
+ strlcpy(gp.numb, er->tokens[1].u.string, GSMD_PB_NUMB_MAXLEN+1);
gp.type = er->tokens[2].u.numeric;
- strcpy(gp.text, er->tokens[3].u.string);
+ strlcpy(gp.text, er->tokens[3].u.string, GSMD_PB_TEXT_MAXLEN+1);
}
else {
DEBUGP("Invalid Input : Parse error\n");
@@ -1231,9 +1234,9 @@
*/
gps.pb.index = er->tokens[0].u.numeric;
- strcpy(gps.pb.numb, er->tokens[1].u.string);
+ strlcpy(gps.pb.numb, er->tokens[1].u.string, GSMD_PB_NUMB_MAXLEN+1);
gps.pb.type = er->tokens[2].u.numeric;
- strcpy(gps.pb.text, er->tokens[3].u.string);
+ strlcpy(gps.pb.text, er->tokens[3].u.string, GSMD_PB_TEXT_MAXLEN+1);
}
else {
DEBUGP("Invalid Input : Parse error\n");
Index: src/gsmd/strl.c
===================================================================
--- src/gsmd/strl.c (revision 0)
+++ src/gsmd/strl.c (revision 0)
@@ -0,0 +1,86 @@
+/* safe strcpy and strcat versions
+ *
+ * Copyright (C) 1991, 1992 Linus Torvalds
+ * Copyright (C) 2008 by Paulius Zaleckas, JSC Teltonika
+ * All Rights Reserved
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <string.h>
+
+#include "gsmd.h"
+#include <gsmd/gsmd.h>
+
+/**
+ * strlcpy - Copy a %NUL terminated string into a sized buffer
+ * @dest: Where to copy the string to
+ * @src: Where to copy the string from
+ * @size: size of destination buffer
+ *
+ * Compatible with *BSD: the result is always a valid
+ * NUL-terminated string that fits in the buffer (unless,
+ * of course, the buffer size is zero). It does not pad
+ * out the result like strncpy() does.
+ */
+size_t strlcpy(char *dest, const char *src, size_t size)
+{
+ size_t ret = strlen(src);
+
+ if (size) {
+ size_t len;
+ if (ret >= size) {
+ len = size - 1;
+ DEBUGP("\"%s\" was truncated by %i characters\n", src,
+ ret - len);
+ }
+ else
+ len = ret;
+ memcpy(dest, src, len);
+ dest[len] = '\0';
+ }
+ return ret;
+}
+
+/**
+ * strlcat - Append a length-limited, %NUL-terminated string to another
+ * @dest: The string to be appended to
+ * @src: The string to append to it
+ * @count: The size of the destination buffer.
+ */
+size_t strlcat(char *dest, const char *src, size_t count)
+{
+ size_t dsize = strlen(dest);
+ size_t len = strlen(src);
+ size_t res = dsize + len;
+
+ /* This would be a bug */
+ if (dsize >= count) {
+ DEBUGP("Length of destination string > provided buffer size!\n");
+ return 0;
+ }
+
+ dest += dsize;
+ count -= dsize;
+ if (len >= count) {
+ len = count - 1;
+ DEBUGP("\"%s\" was truncated by %i characters\n", src,
+ res - len);
+ }
+ memcpy(dest, src, len);
+ dest[len] = 0;
+ return res;
+}
+
Index: src/gsmd/atcmd.c
===================================================================
--- src/gsmd/atcmd.c (revision 4048)
+++ src/gsmd/atcmd.c (working copy)
@@ -619,7 +619,7 @@
atcmd->cb = cb;
atcmd->resp = NULL;
atcmd->timeout = NULL;
- strncpy(atcmd->buf, cmd, buflen-1);
+ strlcpy(atcmd->buf, cmd, buflen);
if (!ct)
atcmd->create_timer_func = discard_timer;
Index: src/gsmd/Makefile.am
===================================================================
--- src/gsmd/Makefile.am (revision 4048)
+++ src/gsmd/Makefile.am (working copy)
@@ -18,7 +18,7 @@
gsmd_CFLAGS = -D PLUGINDIR=\"$(plugindir)\"
gsmd_SOURCES = gsmd.c atcmd.c select.c machine.c vendor.c unsolicited.c log.c \
usock.c talloc.c timer.c operator_cache.c ext_response.c \
- sms_cb.c sms_pdu.c
+ sms_cb.c sms_pdu.c strl.c
gsmd_LDADD = -ldl
gsmd_LDFLAGS = -Wl,--export-dynamic
Index: src/gsmd/vendor_tihtc.c
===================================================================
--- src/gsmd/vendor_tihtc.c (revision 4048)
+++ src/gsmd/vendor_tihtc.c (working copy)
@@ -85,7 +85,7 @@
char *tok1, *tok2;
char tx_buf[20];
- strcpy(tx_buf, buf);
+ strlcpy(tx_buf, buf, sizeof(tx_buf));
tok1 = strtok(tx_buf, ",");
if (!tok1)
return -EIO;
@@ -132,7 +132,7 @@
sizeof(*aux));
char tx_buf[64];
- strcpy(tx_buf, buf);
+ strlcpy(tx_buf, buf, sizeof(tx_buf));
DEBUGP("entering cpi_parse param=`%s'\n", param);
if (!ucmd)
return -EINVAL;
Index: src/gsmd/vendor_ti.c
===================================================================
--- src/gsmd/vendor_ti.c (revision 4048)
+++ src/gsmd/vendor_ti.c (working copy)
@@ -68,7 +68,7 @@
char *tok1, *tok2;
char tx_buf[20];
- strcpy(tx_buf, buf);
+ strlcpy(tx_buf, buf, sizeof(tx_buf));
tok1 = strtok(tx_buf, ",");
if (!tok1)
return -EIO;
@@ -122,7 +122,7 @@
sizeof(*aux));
char tx_buf[64];
- strcpy(tx_buf, buf);
+ strlcpy(tx_buf, buf, sizeof(tx_buf));
DEBUGP("entering cpi_parse param=`%s'\n", param);
if (!ucmd)
return -EINVAL;
Index: src/gsmd/gsmd.c
===================================================================
--- src/gsmd/gsmd.c (revision 4048)
+++ src/gsmd/gsmd.c (working copy)
@@ -152,7 +152,7 @@
struct gsmd *g = ctx;
DEBUGP("imsi : %s\n", resp);
- strcpy(g->imsi, resp);
+ strlcpy(g->imsi, resp, sizeof(g->imsi));
return 0;
}
