[PATCH] bug# 000117: SMSBox crashes if %A is used in get-url

"Angel Fradejas" <[email protected]>
Newsgroups gmane.comp.mobile.kannel.devel
Message-ID <[email protected]>
Forwarded to list for general review.
 
Bug report by Bruno:
%A is the text sent by SMSC when a Delivery Report arrives and doesn't make
sense in a get-url url, but nevertheless I've been reported that a %A in a
get-url does crash kannel. 
 
 
My review:
Tested and confirmed, there is no check for null in url_get_pattern() for
the %A token. 
It is octstr_url_encode() which finally segfaults. 

I propose patches in two flavors to fix this: 

1) the direct approach, patching urltrans.c, this fixes the bug without
additional concerns. 

2) a more "deep" patch, avoiding that
octstr_(url_encode|url_decode|append|insert) segfault when receiving a null
Octstr* 

I think both two are very straightforward. 

Angel FRADEJAS
Mediafusión España, S.A.
[email protected]
www.mediafusion.es <http://www.mediafusion.es/> 
Tel. +34 91 252 3200
Fax  +34 91 252 5969
octstr_nulls.diff (application/octet-stream, 899 B)
Index: gwlib/octstr.c
===================================================================
RCS file: /home/cvs/gateway/gwlib/octstr.c,v
retrieving revision 1.157
diff -u -r1.157 octstr.c
--- gwlib/octstr.c	23 Jan 2004 11:46:36 -0000	1.157
+++ gwlib/octstr.c	3 Mar 2004 09:05:02 -0000
@@ -1168,6 +1168,8 @@
 
 void octstr_insert(Octstr *ostr1, const Octstr *ostr2, long pos)
 {
+    if (ostr2 == NULL)
+        return;
     seems_valid(ostr1);
     seems_valid(ostr2);
     gw_assert(pos <= ostr1->len);
@@ -1637,6 +1639,8 @@
     int all_safe;
     unsigned char c, *str, *str2, *res, *hexits;
 
+    if (ostr == NULL)
+        return;
     seems_valid(ostr);
 
     if (ostr->immutable || ostr->len == 0)
@@ -1706,6 +1710,8 @@
     unsigned char *dptr = ostr->data;
     int code, code2, ret = 0;
 
+    if (ostr == NULL)
+        return;
     seems_valid(ostr);
     gw_assert(!ostr->immutable);
urltrans_pattern_a.diff (application/octet-stream, 742 B)
Index: gw/urltrans.c
===================================================================
RCS file: /home/cvs/gateway/gw/urltrans.c,v
retrieving revision 1.92
diff -u -r1.92 urltrans.c
--- gw/urltrans.c	23 Jan 2004 14:56:06 -0000	1.92
+++ gw/urltrans.c	3 Mar 2004 09:06:21 -0000
@@ -585,11 +585,13 @@
 	    break;
 
 	case 'A':
-	    enc = octstr_duplicate(reply);
-	    octstr_url_encode(enc);
-	    octstr_append(result, enc);
-	    octstr_destroy(enc);
-	    break;
+	    if (reply) {
+            enc = octstr_duplicate(reply);
+            octstr_url_encode(enc);
+            octstr_append(result, enc);
+            octstr_destroy(enc);
+        }
+        break;
 
 	case 'c':
 	    octstr_append_decimal(result, request->sms.coding);
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.