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

Module: sip-router Branch: 3.1 Commit: a1b10bff76e1a88c647612c30b12eb5e9e51c90e URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=a1b10bf… Author: Daniel-Constantin Mierla &lt;miconda(a)gmail.com&gt; Committer: Daniel-Constantin Mierla &lt;miconda(a)gmail.com&gt; 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 --- 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); }