RE: [RFC] timeline for stable 1.2.0

"Oded Arbel" <[email protected]> Sun, 2 Jun 2002 15:17:41 +0300

Newsgroups	gmane.comp.mobile.kannel.devel
Message-ID	<[email protected]>

comes to think about it - it's stupid of me to submit the whole huge patch and ask for special treatment of just one part.
Please ignore my previous email and commit the following patch, which only deals with the memory bugs, to the CVS before doing 1.2.0

TIA

--
Oded Arbel
m-Wise Inc.
[email protected]
(972)-67-340014
(972)-9-9581711 (ext: 116)

::..
The safest seat in a aeroplane is within the black box.


> -----Original Message-----
> From: Oded Arbel 
> Sent: Sunday, June 02, 2002 1:38 PM
> To: [email protected]
> Subject: RE: [RFC] timeline for stable 1.2.0
> 
> 
> 
> > -----Original Message-----
> > From: Stipe Tolj [mailto:[email protected]]
> > Sent: Sunday, June 02, 2002 10:48 AM
> > To: [email protected]
> > Subject: [RFC] timeline for stable 1.2.0
> > 
> > 
> > Hi all,
> > 
> > can we do some voting for releasing the stable version 
> 1.2.0 from the
> > current cvs head tree, please?!
> > 
> > I'd like to hear votes from anyone who is a) core developer, b) core
> > productive user of Kannel and vetos from anyone who see 
> major bugs in
> > current cvs tree.
> > 
> > If there is noone claiming to have segmenation faults or similar on
> > production environments running cvs and we can get rid of the
> > outstanding release showstopper from STATUS (Stefan, comments
> > please?!) then I'd like to suggest releasing 1.2.0 stable upcoming
> > week.
> 
> We at m-Wise encountered a crash problem when sending 
> messages using SMPP. we tracked it to the new 
> validity/defered patch which uses a too short a buffer for 
> composing the time stamp. Attached is a patch to fix that, as 
> well as cleaning up handling of error messages in NACK 
> responses - something which was a major issue for us as it 
> caused us to fail a conformity test.
> 
> The problem with the buffer that it is staticly defined as 16 
> byte long, while 17 bytes are needed (including the 
> terminating NULL) to store the time stamp. sprintf is used to 
> fill the buffer, which always appends the terminating NULL, 
> over flowing the buffer and rewriting something (in our case  
> - the memory location holding the pointer to the PDU). The 
> fix we did is to use Octstr to store the buffer and to build 
> it using octstr_format.
> 
> I'd like to get at least the buffer overflow patch to be 
> submitted before 1.2 as it's a major show stopper.
> 
> --
> Oded Arbel
> m-Wise Inc.
> [email protected]
> (972)-67-340014
> (972)-9-9581711 (ext: 116)
> 
> ::..
> This book fills a much-needed gap.
> 	-- Moses Hadas in a review
>

smsc_smpp.patch (application/octet-stream, 3.2 KB)

Index: gateway/gw/smsc_smpp.c
===================================================================
RCS file: /home/cvs/gateway/gw/smsc_smpp.c,v
retrieving revision 1.69
diff -u -r1.69 smsc_smpp.c
--- gateway/gw/smsc_smpp.c	29 May 2002 15:24:10 -0000	1.69
+++ gateway/gw/smsc_smpp.c	2 Jun 2002 12:14:28 -0000
@@ -237,7 +237,7 @@
 static SMPP_PDU *msg_to_pdu(SMPP *smpp, Msg *msg) 
 { 
     SMPP_PDU *pdu; 
-    char buffer[16];
+    Octstr *buffer;
     Octstr *relation_UTC_time = NULL;
     struct tm gmtime, localtime, tm;
     int gwqdiff;
@@ -327,30 +327,32 @@
         /* work out 1/4 hour difference between local time and UTC/GMT */
         gmtime = gw_gmtime(time(NULL));
         localtime = gw_localtime(time(NULL));
-        if (localtime.tm_hour >= gmtime.tm_hour) {
+        gwqdiff = ((localtime.tm_hour - gmtime.tm_hour) * 4)
+                  + ((localtime.tm_min - gmtime.tm_min) / 15);
+        if (gwqdiff >= 0)
             relation_UTC_time = octstr_create("+");
-            gwqdiff = (localtime.tm_hour - gmtime.tm_hour) * 4;
-        } else {
+	else
             relation_UTC_time = octstr_create("-");
-            gwqdiff = (gmtime.tm_hour - localtime.tm_hour) * 4;
-        }
+
 
         if (msg->sms.validity) {
             tm = gw_localtime(time(NULL) + msg->sms.validity * 60);
-            sprintf(buffer, "%02d%02d%02d%02d%02d%02d0%01d%02d%1s",
+            buffer = octstr_format("%02d%02d%02d%02d%02d%02d0%01d%02d%1s",
                     tm.tm_year % 100, tm.tm_mon + 1, tm.tm_mday,
                     tm.tm_hour, tm.tm_min, tm.tm_sec,
                     0, gwqdiff, octstr_get_cstr(relation_UTC_time));
-            pdu->u.submit_sm.validity_period = octstr_create(buffer);
+            pdu->u.submit_sm.validity_period = octstr_copy(buffer,0,16);
+            octstr_destroy(buffer);
         }
 
         if (msg->sms.deferred) {
             tm = gw_localtime(time(NULL) + msg->sms.deferred * 60);
-            sprintf(buffer, "%02d%02d%02d%02d%02d%02d0%01d%02d%1s",
+            buffer = octstr_format("%02d%02d%02d%02d%02d%02d0%01d%02d%1s",
                     tm.tm_year % 100, tm.tm_mon + 1, tm.tm_mday,
                     tm.tm_hour, tm.tm_min, tm.tm_sec,
                     0, gwqdiff, octstr_get_cstr(relation_UTC_time));
-            pdu->u.submit_sm.schedule_delivery_time = octstr_create(buffer);
+            pdu->u.submit_sm.schedule_delivery_time = octstr_copy(buffer,0,16);
+            octstr_destroy(buffer);
         }
     }
 
@@ -376,7 +378,8 @@
     pdu = smpp_pdu_create(enquire_link, counter_increase(smpp->message_id_counter)); 
     dump_pdu("Sending enquire link:", smpp->conn->id, pdu); 
     os = smpp_pdu_pack(pdu); 
-    conn_write(conn, os); /* Write errors checked by caller. */ 
+    if (os)
+	conn_write(conn, os); /* Write errors checked by caller. */ 
     octstr_destroy(os); 
     smpp_pdu_destroy(pdu); 
 } 
@@ -389,7 +392,10 @@
      
     dump_pdu("Sending PDU:", id, pdu); 
     os = smpp_pdu_pack(pdu); 
-    ret = conn_write(conn, os);   /* Caller checks for write errors later */ 
+    if (os)
+        ret = conn_write(conn, os);   /* Caller checks for write errors later */ 
+    else
+	ret = -1;
     octstr_destroy(os); 
     return ret; 
 }