[sr-dev] git:master: rls(k): safer build of chunked body

Daniel-Constantin Mierla miconda at gmail.com
Thu Apr 7 13:37:33 CEST 2011


Module: sip-router
Branch: master
Commit: 26d8a3fbfbc519535b31b3bae7b5c57d89c2652a
URL:    http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=26d8a3fbfbc519535b31b3bae7b5c57d89c2652a

Author: Daniel-Constantin Mierla <miconda at gmail.com>
Committer: Daniel-Constantin Mierla <miconda at gmail.com>
Date:   Thu Apr  7 13:32:45 2011 +0200

rls(k): safer build of chunked body

- the check for the size of alloc'ed buffer was using static estimation,
  not it is cmputed based on values and makes sure the ending '\0' is
  safe as well. When handling bodies with long values, could have caused
  overflow
- reported by Peter Dunkley
(cherry picked from commit a1b10bff76e1a88c647612c30b12eb5e9e51c90e)

---

 modules_k/rls/notify.c          |   50 +++++++++++++++++++++++---------------
 modules_k/rls/notify.h          |    1 -
 modules_k/rls/resource_notify.c |   37 +++++++++++++++++++---------
 3 files changed, 55 insertions(+), 33 deletions(-)

diff --git a/modules_k/rls/notify.c b/modules_k/rls/notify.c
index d414eeb..06c4ed3 100644
--- a/modules_k/rls/notify.c
+++ b/modules_k/rls/notify.c
@@ -472,11 +472,12 @@ str* constr_multipart_body(db1_res_t* result, char** cid_array,
 	int i, length= 0;
 	db_row_t *row;	
 	db_val_t *row_vals;
-	char* content_id= NULL;
+	str content_id = {0, 0};
 	str body= {0, 0};
-	str ctype= {0, 0};
-	int antet_len;
+	str content_type= {0, 0};
+	int chunk_len;
 	str* multi_body= NULL;
+	str bstr = {0, 0};
 	
 	LM_DBG("start\n");
 	buf= pkg_malloc(size* sizeof(char));
@@ -485,7 +486,8 @@ str* constr_multipart_body(db1_res_t* result, char** cid_array,
 		ERR_MEM(PKG_MEM_STR);
 	}
 
-	antet_len= COMPUTE_ANTET_LEN (boundary_string);
+	bstr.s = boundary_string;
+	bstr.len = strlen(bstr.s);
 
 	for(i= 0; i< result->n; i++)
 	{
@@ -498,29 +500,37 @@ str* constr_multipart_body(db1_res_t* result, char** cid_array,
 		body.s= (char*)row_vals[pres_state_col].val.string_val;
 		body.len= strlen(body.s);
 		trim(&body);
-		ctype.s = (char*)row_vals[content_type_col].val.string_val;
-		ctype.len = strlen(ctype.s);
-
-		if(length+ antet_len+ body.len+ 4 > size)
-		{
-			REALLOC_BUF
-		}
-
-		length+= sprintf(buf+ length, "--%s\r\n\r\n", boundary_string);
-		length+= sprintf(buf+ length, "Content-Transfer-Encoding: binary\r\n");
-		content_id= cid_array[i];
-		if(content_id== NULL)
+		content_type.s = (char*)row_vals[content_type_col].val.string_val;
+		content_type.len = strlen(content_type.s);
+		content_id.s= cid_array[i];
+		if(content_id.s== NULL)
 		{
 			LM_ERR("No cid found in array for uri= %s\n",
 					row_vals[resource_uri_col].val.string_val);
 			goto error;
 		}
+		content_id.len = strlen(content_id.s);
+
 
-		length+= sprintf(buf+ length, "Content-ID: <%s>\r\n",content_id);
+		chunk_len = 4 + bstr.len
+					+ 35
+					+ 16 + content_id.len
+					+ 18 + content_type.len
+					+ 4 + body.len + 8;
+		if(length + chunk_len >= size)
+		{
+			REALLOC_BUF
+		}
+
+		length+= sprintf(buf+ length, "--%.*s\r\n",
+				bstr.len, bstr.s);
+		length+= sprintf(buf+ length, "Content-Transfer-Encoding: binary\r\n");
+		length+= sprintf(buf+ length, "Content-ID: <%.*s>\r\n",
+				content_id.len, content_id.s);
 		length+= sprintf(buf+ length, "Content-Type: %.*s\r\n\r\n",
-				ctype.len, ctype.s);
-		
-		length+= sprintf(buf+length,"%.*s\r\n\r\n", body.len, body.s);
+				content_type.len, content_type.s);
+		length+= sprintf(buf+length,"%.*s\r\n\r\n",
+				body.len, body.s);
 	}
 
 	if(length+ strlen( boundary_string)+ 7> size )
diff --git a/modules_k/rls/notify.h b/modules_k/rls/notify.h
index e189e63..764ae5e 100644
--- a/modules_k/rls/notify.h
+++ b/modules_k/rls/notify.h
@@ -46,7 +46,6 @@
 		if(buf== NULL) \
 		{	ERR_MEM("constr_multipart_body");}
 
-#define COMPUTE_ANTET_LEN(boundary_string) (strlen( boundary_string)+ MAX_HEADERS_LENGTH + 6)
 int send_full_notify(subs_t* subs, xmlNodePtr rl_node, 
 		int version, str* rl_uri, unsigned int hash_code);
 
diff --git a/modules_k/rls/resource_notify.c b/modules_k/rls/resource_notify.c
index 0caf7cb..f93ebe0 100644
--- a/modules_k/rls/resource_notify.c
+++ b/modules_k/rls/resource_notify.c
@@ -482,8 +482,11 @@ void timer_send_notify(unsigned int ticks,void *param)
 	unsigned int hash_code= 0;
 	int len;
 	int size= BUF_REALLOC_SIZE, buf_len= 0;	
-	char* buf= NULL, *auth_state= NULL, *boundary_string= NULL, *cid= NULL;
-	int contor= 0, auth_state_flag, antet_len;
+	char* buf= NULL, *auth_state= NULL, *boundary_string= NULL;
+	str cid = {0,0};
+	str content_type = {0,0};
+	int contor= 0, auth_state_flag;
+	int chunk_len;
 	str bstr= {0, 0};
 	str rlmi_cont= {0, 0}, multi_cont;
 	subs_t* s, *dialog= NULL;
@@ -557,7 +560,6 @@ void timer_send_notify(unsigned int ticks,void *param)
 		ERR_MEM(PKG_MEM_STR);
 	}
 
-	antet_len= COMPUTE_ANTET_LEN(bstr.s);
 	LM_DBG("found %d records with updated state\n", result->n);
 	for(i= 0; i< result->n; i++)
 	{
@@ -686,7 +688,8 @@ void timer_send_notify(unsigned int ticks,void *param)
 		while(1)
 		{
 			contor++;
-			cid= NULL;
+			cid.s= NULL;
+			cid.len= 0;
 			instance_node= xmlNewChild(resource_node, NULL, 
 					BAD_CAST "instance", NULL);
 			if(instance_node== NULL)
@@ -707,8 +710,8 @@ void timer_send_notify(unsigned int ticks,void *param)
 		
 			if(auth_state_flag & ACTIVE_STATE)
 			{
-				cid= generate_cid(resource_uri, strlen(resource_uri));
-				xmlNewProp(instance_node, BAD_CAST "cid", BAD_CAST cid);
+				cid.s= generate_cid(resource_uri, strlen(resource_uri));
+				xmlNewProp(instance_node, BAD_CAST "cid", BAD_CAST cid.s);
 			}
 			else
 			if(auth_state_flag & TERMINATED_STATE)
@@ -718,18 +721,28 @@ void timer_send_notify(unsigned int ticks,void *param)
 			}
 		
 			/* add in the multipart buffer */
-			if(cid)
+			if(cid.s)
 			{
-				if(buf_len+ antet_len+ pres_state.len+ 4 > size)
+				cid.len = strlen(cid.s);
+				content_type.s = (char*)row_vals[content_type_col].val.string_val;
+				content_type.len = strlen(content_type.s);
+				chunk_len = 4 + bstr.len
+							+ 35
+							+ 16 + cid.len
+							+ 18 + content_type.len
+							+ 4 + pres_state.len + 8;
+				if(buf_len + chunk_len >= size)
 				{
 					REALLOC_BUF
 				}
-				buf_len+= sprintf(buf+ buf_len, "--%s\r\n", bstr.s);
+				buf_len+= sprintf(buf+ buf_len, "--%.*s\r\n", bstr.len,
+						bstr.s);
 				buf_len+= sprintf(buf+ buf_len,
 						"Content-Transfer-Encoding: binary\r\n");
-				buf_len+= sprintf(buf+ buf_len, "Content-ID: <%s>\r\n", cid);
-				buf_len+= sprintf(buf+ buf_len, "Content-Type: %s\r\n\r\n",  
-						row_vals[content_type_col].val.string_val);
+				buf_len+= sprintf(buf+ buf_len, "Content-ID: <%.*s>\r\n",
+						cid.len, cid.s);
+				buf_len+= sprintf(buf+ buf_len, "Content-Type: %.*s\r\n\r\n",
+						content_type.len, content_type.s);
 				buf_len+= sprintf(buf+buf_len,"%.*s\r\n\r\n", pres_state.len,
 						pres_state.s);
 			}




More information about the sr-dev mailing list