瀏覽代碼

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)
Daniel-Constantin Mierla 14 年之前
父節點
當前提交
26d8a3fbfb
共有 3 個文件被更改,包括 55 次插入33 次删除
  1. 30 20
      modules_k/rls/notify.c
  2. 0 1
      modules_k/rls/notify.h
  3. 25 12
      modules_k/rls/resource_notify.c

+ 30 - 20
modules_k/rls/notify.c

@@ -472,11 +472,12 @@ str* constr_multipart_body(db1_res_t* result, char** cid_array,
 	int i, length= 0;
 	int i, length= 0;
 	db_row_t *row;	
 	db_row_t *row;	
 	db_val_t *row_vals;
 	db_val_t *row_vals;
-	char* content_id= NULL;
+	str content_id = {0, 0};
 	str body= {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* multi_body= NULL;
+	str bstr = {0, 0};
 	
 	
 	LM_DBG("start\n");
 	LM_DBG("start\n");
 	buf= pkg_malloc(size* sizeof(char));
 	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);
 		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++)
 	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.s= (char*)row_vals[pres_state_col].val.string_val;
 		body.len= strlen(body.s);
 		body.len= strlen(body.s);
 		trim(&body);
 		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",
 			LM_ERR("No cid found in array for uri= %s\n",
 					row_vals[resource_uri_col].val.string_val);
 					row_vals[resource_uri_col].val.string_val);
 			goto error;
 			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",
 		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 )
 	if(length+ strlen( boundary_string)+ 7> size )

+ 0 - 1
modules_k/rls/notify.h

@@ -46,7 +46,6 @@
 		if(buf== NULL) \
 		if(buf== NULL) \
 		{	ERR_MEM("constr_multipart_body");}
 		{	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 send_full_notify(subs_t* subs, xmlNodePtr rl_node, 
 		int version, str* rl_uri, unsigned int hash_code);
 		int version, str* rl_uri, unsigned int hash_code);
 
 

+ 25 - 12
modules_k/rls/resource_notify.c

@@ -482,8 +482,11 @@ void timer_send_notify(unsigned int ticks,void *param)
 	unsigned int hash_code= 0;
 	unsigned int hash_code= 0;
 	int len;
 	int len;
 	int size= BUF_REALLOC_SIZE, buf_len= 0;	
 	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 bstr= {0, 0};
 	str rlmi_cont= {0, 0}, multi_cont;
 	str rlmi_cont= {0, 0}, multi_cont;
 	subs_t* s, *dialog= NULL;
 	subs_t* s, *dialog= NULL;
@@ -557,7 +560,6 @@ void timer_send_notify(unsigned int ticks,void *param)
 		ERR_MEM(PKG_MEM_STR);
 		ERR_MEM(PKG_MEM_STR);
 	}
 	}
 
 
-	antet_len= COMPUTE_ANTET_LEN(bstr.s);
 	LM_DBG("found %d records with updated state\n", result->n);
 	LM_DBG("found %d records with updated state\n", result->n);
 	for(i= 0; i< result->n; i++)
 	for(i= 0; i< result->n; i++)
 	{
 	{
@@ -686,7 +688,8 @@ void timer_send_notify(unsigned int ticks,void *param)
 		while(1)
 		while(1)
 		{
 		{
 			contor++;
 			contor++;
-			cid= NULL;
+			cid.s= NULL;
+			cid.len= 0;
 			instance_node= xmlNewChild(resource_node, NULL, 
 			instance_node= xmlNewChild(resource_node, NULL, 
 					BAD_CAST "instance", NULL);
 					BAD_CAST "instance", NULL);
 			if(instance_node== NULL)
 			if(instance_node== NULL)
@@ -707,8 +710,8 @@ void timer_send_notify(unsigned int ticks,void *param)
 		
 		
 			if(auth_state_flag & ACTIVE_STATE)
 			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
 			else
 			if(auth_state_flag & TERMINATED_STATE)
 			if(auth_state_flag & TERMINATED_STATE)
@@ -718,18 +721,28 @@ void timer_send_notify(unsigned int ticks,void *param)
 			}
 			}
 		
 		
 			/* add in the multipart buffer */
 			/* 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
 					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,
 				buf_len+= sprintf(buf+ buf_len,
 						"Content-Transfer-Encoding: binary\r\n");
 						"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,
 				buf_len+= sprintf(buf+buf_len,"%.*s\r\n\r\n", pres_state.len,
 						pres_state.s);
 						pres_state.s);
 			}
 			}