modules_k/rls: Memory allocation problem for NOTIFY bodies

- A few problems here:
  - The actual allocated buffer size was not used in the calculations in
    constr_multipart_body
  - The buffer pointer was copied to a local variable and not copied back after
    a realloc in constr_mulitpart_body().  This resulted in a double free when
    the realloc moved the buffer.
  - The length of the data in string buffer was never copied back to the str in
    constr_multipart_body().
  - In both constr_multipart_body() and resource_notify.c when the buffer does
    not contain enough space another 2048 bytes is allocated.  This is
    regardless of how much more space is actually needed.  So if 4096 bytes
    were needed 2048 bytes would be allocated and then the end of the buffer
    would be overwritten by 4096 bytes of data.
  - Problem found and diagnosed during testing at Crocodile RCS
  - Fix implemented by Hugh Waite @ Crocodile RCS

modules_k/rls/notify.c

@@ -51,6 +51,9 @@
 #include <libxml/xpath.h>
 #include <libxml/xpathInternals.h>
 
+static str *multipart_body = NULL;
+static int multipart_body_size = 0;
+
 typedef struct res_param
 {
     struct uri_link **next;
@@ -68,7 +71,7 @@ int resource_uri_col=0, content_type_col, pres_state_col= 0,
 xmlDocPtr constr_rlmi_doc(db1_res_t* result, str* rl_uri, int version,
 		xmlNodePtr rl_node, char*** cid_array,
 		str username, str domain);
-void constr_multipart_body(str *multipart_body, const str *const content_type, const str *const body, str *cid, int boundary_len, char *boundary_string);
+void constr_multipart_body(const str *const content_type, const str *const body, str *cid, int boundary_len, char *boundary_string);
 
 dlg_t* rls_notify_dlg(subs_t* subs);
 
@@ -78,14 +81,13 @@ int parse_xcap_uri(char *uri, str *host, unsigned short *port, str *path);
 int rls_get_resource_list(str *rl_uri, str *username, str *domain,
 		xmlNodePtr *rl_node, xmlDocPtr *xmldoc);
 int add_resource_to_list(char* uri, void* param);
-int add_resource(char* uri, xmlNodePtr list_node, str *multipart_body, char * boundary_string, db1_res_t *result, int *len_est);
+int add_resource(char* uri, xmlNodePtr list_node, char * boundary_string, db1_res_t *result, int *len_est);
 
 int send_full_notify(subs_t* subs, xmlNodePtr rl_node, str* rl_uri,
 		unsigned int hash_code)
 {
 	xmlDocPtr rlmi_body= NULL;
     xmlNodePtr list_node= NULL;
-	str* multipart_body= NULL;
 	db_key_t query_cols[2], update_cols[2], result_cols[7];
 	db_val_t query_vals[2], update_vals[2];
 	db1_res_t *result= NULL;
@@ -96,8 +98,6 @@ int send_full_notify(subs_t* subs, xmlNodePtr rl_node, str* rl_uri,
     uri_link_t *uri_list_head = NULL;
     int len_est;
     res_param_t param;
-    char* body_buffer;
-    int size= BUF_REALLOC_SIZE;
     int resource_added = 0; /* Flag to indicate that we have added at least one resource */
 
 	LM_DBG("start\n");
@@ -137,19 +137,20 @@ int send_full_notify(subs_t* subs, xmlNodePtr rl_node, str* rl_uri,
 
     /* Allocate an initial buffer for the multipart body.
 	 * This buffer will be reallocated if neccessary */
-    body_buffer= (char *)pkg_malloc(size);
-	if(body_buffer== NULL)
+	multipart_body= (str*)pkg_malloc(sizeof(str));
+	if(multipart_body== NULL)
 	{
 		ERR_MEM(PKG_MEM_STR);
 	}
-    
-    multipart_body= (str*)pkg_malloc(sizeof(str));
-	if(multipart_body== NULL)
+
+    multipart_body_size = BUF_REALLOC_SIZE;
+    multipart_body->s = (char *)pkg_malloc(multipart_body_size);
+
+	if(multipart_body->s== NULL)
 	{
 		ERR_MEM(PKG_MEM_STR);
 	}
-
-	multipart_body->s= body_buffer;
+    
 	multipart_body->len= 0;
 
     /* Create an empty rlmi document */
@@ -169,7 +170,7 @@ int send_full_notify(subs_t* subs, xmlNodePtr rl_node, str* rl_uri,
     while (uri_list_head)
 	{
         uri_link_t *last = uri_list_head;
-        if (add_resource(uri_list_head->uri, list_node, multipart_body, boundary_string, result, &len_est) >0)
+        if (add_resource(uri_list_head->uri, list_node, boundary_string, result, &len_est) >0)
         {
             if (resource_added == 0)
             {
@@ -257,9 +258,11 @@ int send_full_notify(subs_t* subs, xmlNodePtr rl_node, str* rl_uri,
 
 	if(multipart_body)			
 	{
-		pkg_free(multipart_body->s);
+	    if (multipart_body->s)
+			pkg_free(multipart_body->s);
 		pkg_free(multipart_body);
 	}
+	multipart_body_size = 0;
 	pkg_free(rlsubs_did.s);
 
 	return 0;
@@ -277,6 +280,7 @@ error:
 			pkg_free(multipart_body->s);
 		pkg_free(multipart_body);
 	}
+	multipart_body_size = 0;
 	
 	if(result)
 		rls_dbf.free_result(rls_db, result);
@@ -369,7 +373,7 @@ error:
 
 
 int add_resource_instance(char* uri, xmlNodePtr resource_node,
-		db1_res_t* result, str *multipart_body, char * boundary_string,
+		db1_res_t* result, char * boundary_string,
         int *len_est)
 {
 	xmlNodePtr instance_node= NULL;
@@ -450,7 +454,7 @@ int add_resource_instance(char* uri, xmlNodePtr resource_node,
 
 			if(auth_state_flag & ACTIVE_STATE)
 			{
-                constr_multipart_body (multipart_body, &content_type, &body, &cid, boundary_len, boundary_string);
+                constr_multipart_body (&content_type, &body, &cid, boundary_len, boundary_string);
 
 				xmlNewProp(instance_node, BAD_CAST "cid", BAD_CAST cid.s);
 			}
@@ -469,7 +473,7 @@ error:
 	return -1;
 }
 
-int add_resource(char* uri, xmlNodePtr list_node, str *multipart_body, char * boundary_string, db1_res_t *result, int *len_est)
+int add_resource(char* uri, xmlNodePtr list_node, char * boundary_string, db1_res_t *result, int *len_est)
 {
 	xmlNodePtr resource_node= NULL;
     int res;
@@ -489,7 +493,7 @@ int add_resource(char* uri, xmlNodePtr list_node, str *multipart_body, char * bo
 	}
 	xmlNewProp(resource_node, BAD_CAST "uri", BAD_CAST uri);
 
-    res = add_resource_instance(uri, resource_node, result, multipart_body, boundary_string, len_est);
+    res = add_resource_instance(uri, resource_node, result, boundary_string, len_est);
 	if(res < 0)
 	{
 		LM_ERR("while adding resource instance node\n");
@@ -587,10 +591,9 @@ error:
 }
 
 
-void constr_multipart_body(str *multipart_body, const str *const content_type, const str *const body, str *cid, int boundary_len, char *boundary_string)
+void constr_multipart_body(const str *const content_type, const str *const body, str *cid, int boundary_len, char *boundary_string)
 {
 	char* buf= multipart_body->s;
-	int size= BUF_REALLOC_SIZE;
 	int length= multipart_body->len;
 	int chunk_len;
 	
@@ -601,10 +604,16 @@ void constr_multipart_body(str *multipart_body, const str *const content_type, c
                 + 16 + cid->len
                 + 18 + content_type->len
                 + 4 + body->len + 8;
-		if(length + chunk_len >= size)
+		while(length + chunk_len >= multipart_body_size)
 		{
-			REALLOC_BUF
+			multipart_body_size += BUF_REALLOC_SIZE;
+			multipart_body->s = (char*)pkg_realloc(multipart_body->s, multipart_body_size);
+			if(multipart_body->s == NULL) 
+			{
+				ERR_MEM("constr_multipart_body");
+			}
 		}
+		buf = multipart_body->s;
 
 		length+= sprintf(buf+ length, "--%.*s\r\n",
             boundary_len, boundary_string);
@@ -616,6 +625,8 @@ void constr_multipart_body(str *multipart_body, const str *const content_type, c
 		length+= sprintf(buf+length,"%.*s\r\n\r\n",
             body->len, body->s);
 
+	multipart_body->len = length;
+
 error:
 
 	return;

modules_k/rls/resource_notify.c

@@ -371,7 +371,7 @@ void send_notifies(db1_res_t *result, int did_col, int resource_uri_col, int aut
 			if(cid.s)
 			{
 	
-				if(buf_len + chunk_len >= size)
+				while(buf_len + chunk_len >= size)
 				{
 					REALLOC_BUF
 				}