Hi,
I've been using kannel since few months ago, looking the code found
possibles buffer overflow.
* fixed size local buffer : they are prime targets for buffer overflow attacks.
* sprintf : the format string could contain `%s' without precision
that could result in a buffer overflow.
* strcpy : if the argument 2 passed to this function copy more data
than can be handled, resulting in a buffer overflow.
I would since today follow looking all source tree and submit the patch.
I hope the list can appreciate it and it to be committed to CVS ;p
--- tmp/gateway/gw/smsc/smsc_emi_x25.c 2005-02-11 11:35:48.000000000 -0400
+++ gateway/gw/smsc/smsc_emi_x25.c 2006-05-10 19:38:00.000000000 -0400
@@ -137,11 +137,16 @@
*/
static int emi_open_connection(SMSCenter *smsc)
{
- char tmpbuff[1024];
+ char *tmpbuff;
+
+ tmpbuff = gw_malloc(1024);
+ memset(tmpbuff, 0, 1024);
- sprintf(tmpbuff, "/dev/%s", smsc->emi_serialdevice);
+ snprintf(tmpbuff, 1024 - 1, "/dev/%s", smsc->emi_serialdevice);
smsc->emi_fd = at_dial(tmpbuff, smsc->emi_phonenum, "ATD", 30);
+ gw_free(tmpbuff);
+
if (smsc->emi_fd <= 0)
return -1;
@@ -170,7 +175,7 @@
if (emi_open_connection(smsc) < 0)
goto error;
- sprintf(smsc->name, "EMI:%s:%s", smsc->emi_phonenum,
+ snprintf(smsc->name, 1024 - 1, "EMI:%s:%s", smsc->emi_phonenum,
smsc->emi_username);
return smsc;
@@ -224,20 +229,33 @@
static int emi_open_session(SMSCenter *smsc)
{
- char message_whole [1024];
- char message_body [1024];
- char message_header [50];
- char message_footer [10];
- char my_buffer [1024];
+ char *message_whole;
+ char *message_body;
+ char *message_header;
+ char *message_footer;
+ char *my_buffer;
int length;
- memset(message_whole, 0, sizeof(message_whole));
- memset(message_body, 0, sizeof(message_body));
- memset(message_header, 0, sizeof(message_header));
- memset(message_footer, 0, sizeof(message_footer));
+ message_whole = gw_malloc(1024);
+ message_body = gw_malloc(1024);
+ message_header = gw_malloc(50);
+ message_footer = gw_malloc(10);
+ my_buffer = gw_malloc(1024);
+
+ memset(message_whole, 0, 1024);
+ memset(message_body, 0, 1024);
+ memset(message_header, 0, 50);
+ memset(message_footer, 0, 10);
+ memset(my_buffer, 0, 1024);
if (emi_fill_ucp60_login(message_body, smsc->emi_username,
smsc->emi_password) < 0) {
error(0, "emi_fill_ucp60_login failed");
+ gw_free(message_whole);
+ gw_free(message_body);
+ gw_free(message_header);
+ gw_free(message_footer);
+ gw_free(my_buffer);
+
return -1;
}
@@ -246,21 +264,27 @@
length += 2; /* footer (fixed) */
length += 2; /* slashes between header, body, footer */
- sprintf(message_header, "%02i/%05i/O/60",
+ snprintf(message_header, 50 - 1, "%02i/%05i/O/60",
(smsc->emi_current_msg_number++ % 100), length);
/* FOOTER */
- sprintf(my_buffer, "%s/%s/", message_header, message_body);
+ snprintf(my_buffer, 1024 - 1, "%s/%s/", message_header, message_body);
generate_checksum(my_buffer, message_footer);
- sprintf(message_whole, "\x02%s/%s/%s\x03", message_header,
+ snprintf(message_whole, 1024 - 1, "\x02%s/%s/%s\x03", message_header,
message_body, message_footer);
debug("bb.sms.emi", 0, "final UCP60 msg: <%s>", message_whole);
put_data(smsc, message_whole, strlen(message_whole), 0);
+ gw_free(message_whole);
+ gw_free(message_body);
+ gw_free(message_header);
+ gw_free(message_footer);
+ gw_free(my_buffer);
+
if (!wait_for_ack(smsc, 60)) {
info(0, "emi_open_session: wait for ack failed!");
return -1;
@@ -471,7 +495,7 @@
static int at_dial(char *device, char *phonenum, char *at_prefix,
time_t how_long)
{
- char tmpbuff[1024];
+ char *tmpbuff;
int howmanyread = 0;
int thistime = 0;
int redial;
@@ -502,6 +526,9 @@
tios.c_cflag |= (HUPCL | CREAD | CRTSCTS);
tcsetattr(fd, TCSANOW, &tios);
+ tmpbuff = gw_malloc(1024);
+ memset(tmpbuff, 0, 1024);
+
/* Dial using an AT command string. */
for (redial = 1; redial; ) {
info(0, "at_dial: dialing <%s> on <%s> for <%i> seconds",
@@ -510,9 +537,9 @@
/* Send AT dial request. */
howmanyread = 0;
- sprintf(tmpbuff, "%s%s\r\n", at_prefix, phonenum);
+ snprintf(tmpbuff, 1024 - 1, "%s%s\r\n", at_prefix, phonenum);
ret = write(fd, tmpbuff, strlen(tmpbuff)); /* errors... -mg */
- memset(&tmpbuff, 0, sizeof(tmpbuff));
+ memset(tmpbuff, 0, 1024);
/* Read the answer to the AT command and react accordingly. */
for (; ; ) {
@@ -521,7 +548,7 @@
goto timeout;
/* We don't need more space for dialout */
- if (howmanyread >= (int) sizeof(tmpbuff))
+ if (howmanyread >= 1024)
goto error;
/* We read 1 char a time so that we don't
@@ -580,15 +607,18 @@
} /* End of dial loop. */
debug("bb.sms.emi", 0, "at_dial: done with dialing");
+ gw_free(tmpbuff);
return fd;
timeout:
error(0, "at_dial timed out");
+ gw_free(tmpbuff);
close(fd);
return -1;
error:
error(0, "at_dial failed");
+ gw_free(tmpbuff);
close(fd);
return -1;
}
@@ -807,9 +837,15 @@
*/
static int memorybuffer_has_rawmessage(SMSCenter *smsc, int type, char auth)
{
- char tmpbuff[1024], tmpbuff2[1024];
+ char *tmpbuff, *tmpbuff2;
char *stx, *etx;
+ tmpbuff = gw_malloc(1024);
+ tmpbuff2 = gw_malloc(1024);
+
+ memset(tmpbuff, 0, 1024);
+ memset(tmpbuff2, 0, 1024);
+
stx = memchr(smsc->buffer, '\2', smsc->buflen);
etx = memchr(smsc->buffer, '\3', smsc->buflen);
@@ -817,15 +853,19 @@
strncpy(tmpbuff, stx, etx - stx + 1);
tmpbuff[etx - stx + 1] = '\0';
if (auth)
- sprintf(tmpbuff2, "/%c/%02i/", auth, type);
+ snprintf(tmpbuff2, 1024 - 1, "/%c/%02i/", auth, type);
else
- sprintf(tmpbuff2, "/%02i/", type);
+ snprintf(tmpbuff2, 1024 - 1, "/%02i/", type);
if (strstr(tmpbuff, tmpbuff2) != NULL) {
debug("bb.sms.emi", 0, "found message <%c/%02i>...msg
<%s>", auth, type, tmpbuff);
+ gw_free(tmpbuff);
+ gw_free(tmpbuff);
return 1;
}
}
+ gw_free(tmpbuff);
+ gw_free(tmpbuff);
return 0;
}
@@ -880,13 +920,14 @@
char emivars[128][1024];
char *leftslash, *rightslash;
- char isotext[2048];
+ char *isotext;
int msgnbr;
int tmpint;
msgnbr = -1;
- memset(isotext, 0, sizeof(isotext));
+ isotext = gw_malloc(2048);
+ memset(isotext, 0, 2048);
strncpy(isotext, rawmessage, length);
leftslash = isotext;
@@ -901,25 +942,25 @@
break;
*rightslash = '\0';
- strcpy(emivars[tmpint], leftslash + 1);
+ strncpy(emivars[tmpint], leftslash + 1, sizeof(emivars[tmpint]) - 1);
leftslash = rightslash;
}
if (strcmp(emivars[3], "01") == 0) {
if (strcmp(emivars[7], "2") == 0) {
- strcpy(isotext, emivars[8]);
+ strncpy(isotext, emivars[8], 2048 - 1);
} else if (strcmp(emivars[7], "3") == 0) {
- parse_emi_to_iso88591(emivars[8], isotext, sizeof(isotext),
+ parse_emi_to_iso88591(emivars[8], isotext, 2048,
smsc->alt_charset);
} else {
error(0, "Unknown 01-type EMI SMS (%s)", emivars[7]);
strcpy(isotext, "");
}
} else if (strcmp(emivars[3], "51") == 0) {
- parse_emi_to_iso88591(emivars[24], isotext, sizeof(isotext),
+ parse_emi_to_iso88591(emivars[24], isotext, 2048,
smsc->alt_charset);
} else if (strcmp(emivars[3], "52") == 0) {
- parse_emi_to_iso88591(emivars[24], isotext, sizeof(isotext),
+ parse_emi_to_iso88591(emivars[24], isotext, 2048,
smsc->alt_charset);
} else {
error(0, "HEY WE SHOULD NOT BE HERE!! Type = %s", emivars[3]);
@@ -934,9 +975,11 @@
(*msg)->sms.msgdata = octstr_create(isotext);
(*msg)->sms.udhdata = NULL;
+ gw_free(isotext);
return msgnbr;
error:
+ gw_free(isotext);
return -1;
}
@@ -948,19 +991,25 @@
{
char emivars[128][1024];
- char timestamp[2048], sender[2048], receiver[2048];
- char emitext[2048], isotext[2048];
+ char *timestamp, *sender, *receiver;
+ char *emitext, *isotext;
char *leftslash, *rightslash;
int msgnbr;
int tmpint;
int is_backup = 0;
+ sender = gw_malloc(2048);
+ receiver = gw_malloc(2048);
+ emitext = gw_malloc(2048);
+ isotext = gw_malloc(2048);
+ timestamp = gw_malloc(2048);
+
msgnbr = -1;
- memset(&sender, 0, sizeof(sender));
- memset(&receiver, 0, sizeof(receiver));
- memset(&emitext, 0, sizeof(emitext));
- memset(&isotext, 0, sizeof(isotext));
- memset(×tamp, 0, sizeof(timestamp));
+ memset(sender, 0, 2048);
+ memset(receiver, 0, 2048);
+ memset(emitext, 0, 2048);
+ memset(isotext, 0, 2048);
+ memset(timestamp, 0, 2048);
strncpy(isotext, rawmessage, length);
leftslash = isotext;
@@ -978,31 +1027,36 @@
break;
*rightslash = '\0';
- strcpy(emivars[tmpint], leftslash + 1);
+ strncpy(emivars[tmpint], leftslash + 1, 1024 - 1);
leftslash = rightslash;
}
/* BODY */
- sprintf(isotext, "A//%s:%s", emivars[4], emivars[18]);
- sprintf(isotext, "A//%s:", emivars[5]);
+ snprintf(isotext, 2048 - 1, "A//%s:%s", emivars[4], emivars[18]);
+ snprintf(isotext, 2048 - 1, "A//%s:", emivars[5]);
is_backup = 0;
/* HEADER */
debug("bb.sms.emi", 0, "acknowledge: type = '%s'", emivars[3]);
- sprintf(emitext, "%s/%05i/%s/%s", emivars[0], (int) strlen(isotext) + 17,
+ snprintf(emitext, 2048 - 1, "%s/%05i/%s/%s", emivars[0], (int)
strlen(isotext) + 17,
"R", emivars[3]);
smsc->emi_current_msg_number = atoi(emivars[0]) + 1;
/* FOOTER */
- sprintf(timestamp, "%s/%s/", emitext, isotext);
+ snprintf(timestamp, 2048 - 1, "%s/%s/", emitext, isotext);
generate_checksum(timestamp, receiver);
- sprintf(sender, "%c%s/%s/%s%c", 0x02, emitext, isotext, receiver, 0x03);
+ snprintf(sender, 2048 - 1, "%c%s/%s/%s%c", 0x02, emitext,
isotext, receiver, 0x03);
put_data(smsc, sender, strlen(sender), is_backup);
+ gw_free(sender);
+ gw_free(receiver);
+ gw_free(emitext);
+ gw_free(isotext);
+ gw_free(timestamp);
return msgnbr;
}
@@ -1013,36 +1067,51 @@
*/
static int parse_msg_to_rawmessage(SMSCenter *smsc, Msg *msg, char
*rawmessage, int rawmessage_length)
{
- char message_whole[10*1024];
- char message_body[10*1024];
- char message_header[1024];
- char message_footer[1024];
-
- char my_buffer[10*1024];
- char my_buffer2[10*1024];
- char msgtext[1024];
+ char *message_whole;
+ char *message_body;
+ char *message_header;
+ char *message_footer;
+
+ char *my_buffer;
+ char *my_buffer2;
+ char *msgtext;
int length;
char mt;
- char mcl[20];
- char snumbits[20];
- char xser[1024];
+ char *mcl;
+ char *snumbits;
+ char *xser;
int udh_len;
- memset(&message_whole, 0, sizeof(message_whole));
- memset(&message_body, 0, sizeof(message_body));
- memset(&message_header, 0, sizeof(message_header));
- memset(&message_footer, 0, sizeof(message_footer));
- memset(&my_buffer, 0, sizeof(my_buffer));
- memset(&my_buffer2, 0, sizeof(my_buffer2));
+ message_whole = gw_malloc(10 * 1024);
+ message_body = gw_malloc(10 * 1024);
+ message_header = gw_malloc(1024);
+ message_footer = gw_malloc(1024);
+ my_buffer = gw_malloc(10 * 1024);
+ my_buffer2 = gw_malloc(10 * 1024);
+ msgtext = gw_malloc(1024);
+ mcl = gw_malloc(20);
+ snumbits = gw_malloc(20);
+ xser = gw_malloc(1024);
+
+ memset(message_whole, 0, 10 * 1024);
+ memset(message_body, 0, 10 * 1024);
+ memset(message_header, 0, 1024);
+ memset(message_footer, 0, 1024);
+ memset(my_buffer, 0, 10 * 1024);
+ memset(my_buffer2, 0, 10 * 1024);
mt = '3';
- memset(&snumbits, 0, sizeof(snumbits));
- memset(&xser, 0, sizeof(xser));
+ memset(msgtext, 0, 1024);
+ memset(mcl, 0, 20);
+ memset(snumbits, 0, 20);
+ memset(xser, 0, 1024);
/* XXX parse_iso88591_to_emi shouldn't use NUL terminated
* strings, but Octstr directly, or a char* and a length.
*/
if (octstr_len(msg->sms.udhdata)) {
- char xserbuf[258];
+ char *xserbuf;
+ xserbuf = gw_malloc(258);
+ memset(xserbuf, 0, 258);
/* we need a properly formated UDH here, there first byte
contains his length
* this will be formatted in the xser field of the EMI Protocol
*/
@@ -1051,6 +1120,7 @@
xserbuf[1] = udh_len;
octstr_get_many_chars(&xserbuf[2], msg->sms.udhdata, 0, udh_len);
parse_binary_to_emi(xserbuf, xser, udh_len + 2);
+ gw_free(xserbuf);
} else {
udh_len = 0;
}
@@ -1070,7 +1140,7 @@
parse_binary_to_emi(msgtext, my_buffer2, octstr_len(msg->sms.msgdata));
- sprintf(snumbits, "%04ld", octstr_len(msg->sms.msgdata)*8);
+ snprintf(snumbits, 20 - 1, "%04ld", octstr_len(msg->sms.msgdata)*8);
mt = '4';
strcpy(mcl, "1");
}
@@ -1079,7 +1149,7 @@
* Please someone encode it with fields_to_dcs
*/
- sprintf(message_body,
+ snprintf(message_body, 10 * 1024 - 1,
"%s/%s/%s/%s/%s//%s////////////%c/%s/%s////%s//////%s//",
octstr_get_cstr(msg->sms.receiver),
msg->sms.sender ? octstr_get_cstr(msg->sms.sender) : "",
@@ -1100,14 +1170,14 @@
length += 2; /* footer (fixed) */
length += 2; /* slashes between header, body, footer */
- sprintf(message_header, "%02i/%05i/%s/%s",
(smsc->emi_current_msg_number++ % 100), length, "O", "51");
+ snprintf(message_header, 1024 - 1, "%02i/%05i/%s/%s",
(smsc->emi_current_msg_number++ % 100), length, "O", "51");
/* FOOTER */
- sprintf(my_buffer, "%s/%s/", message_header, message_body);
+ snprintf(my_buffer, 10 * 1024 - 1, "%s/%s/", message_header, message_body);
generate_checksum(my_buffer, message_footer);
- sprintf(message_whole, "%c%s/%s/%s%c", 0x02, message_header,
message_body, message_footer, 0x03);
+ snprintf(message_whole, 10 * 1024 - 1, "%c%s/%s/%s%c", 0x02,
message_header, message_body, message_footer, 0x03);
strncpy(rawmessage, message_whole, rawmessage_length);
@@ -1117,6 +1187,17 @@
}
debug("bb.sms.emi", 0, "emi %d message %s",
smsc->emi_current_msg_number, rawmessage);
+
+ gw_free(message_whole);
+ gw_free(message_body);
+ gw_free(message_header);
+ gw_free(message_footer);
+ gw_free(my_buffer);
+ gw_free(my_buffer2);
+ gw_free(msgtext);
+ gw_free(mcl);
+ gw_free(snumbits);
+ gw_free(xser);
return strlen(rawmessage);
}
@@ -1128,7 +1209,10 @@
{
int hmtg = 0;
unsigned int mychar;
- char tmpbuff[128];
+ char *tmpbuff;
+
+ tmpbuff = gw_malloc(128);
+ memset(tmpbuff, 0, 128);
for (hmtg = 0; hmtg <= (int)strlen(from); hmtg += 2) {
strncpy(tmpbuff, from + hmtg, 2);
@@ -1138,6 +1222,7 @@
to[(hmtg / 2)-1] = '\0';
+ gw_free(tmpbuff);
return 0;
}
@@ -1148,22 +1233,25 @@
static int parse_iso88591_to_emi(char *from, char *to,
int length, int alt_charset)
{
- char buf[10];
+ char *buf;
unsigned char tmpchar;
char *ptr;
if (!from || !to || length <= 0)
return -1;
+ buf = gw_malloc(10);
+ memset(buf, 0, 10);
*to = '\0';
debug("bb.sms.emi", 0, "emi parsing <%s> to emi, length %d", from, length);
for (ptr = from; length > 0; ptr++, length--) {
tmpchar = char_iso_to_sms(*ptr, alt_charset);
- sprintf(buf, "%02X", tmpchar);
+ snprintf(buf, 10 - 1, "%02X", tmpchar);
strncat(to, buf, 2);
}
+ gw_free(buf);
return 0;
}
@@ -1172,19 +1260,22 @@
*/
static int parse_binary_to_emi(char *from, char *to, int length)
{
- char buf[10];
+ char *buf;
char *ptr;
if (!from || !to || length <= 0)
return -1;
+ buf = gw_malloc(10);
+ memset(buf, 0, 10);
*to = '\0';
for (ptr = from; length > 0; ptr++, length--) {
- sprintf(buf, "%02X", (unsigned char)*ptr);
+ snprintf(buf, 10 - 1, "%02X", (unsigned char)*ptr);
strncat(to, buf, 2);
}
+ gw_free(buf);
return 0;
}
--
Humberto Figuera - Using Linux 2.6.16
Usuario GNU/Linux 369709
Caracas - Venezuela
GPG Key Fingerprint = 5AAC DF0C 00F4 2834 28BA 37AD 3364 01D1 74CA 0603
