From 6d058ec495c2faa015b24f412e2fe1b55526657e Mon Sep 17 00:00:00 2001 From: Walter Doekes Date: Wed, 12 Mar 2014 10:26:54 +0100 Subject: [PATCH] presence_dialoginfo: Fix bounds checking by using a helper function. Reported by: dsandras > About the bound checking error, the code was cut&pasted from another > place in the same file where the same error is still present: > ... Also I replaced a heap str with a stack one in build_dialoginfo. This one wasn't freed either unless an error condition was hit. --- modules/presence_dialoginfo/notify_body.c | 75 +++++++++-------------- 1 file changed, 28 insertions(+), 47 deletions(-) diff --git a/modules/presence_dialoginfo/notify_body.c b/modules/presence_dialoginfo/notify_body.c index 15d065ca56f..8e1e92e5349 100644 --- a/modules/presence_dialoginfo/notify_body.c +++ b/modules/presence_dialoginfo/notify_body.c @@ -57,22 +57,28 @@ void free_xml_body(char* body) xmlFree(body); } +/* Joins user and domain into "sip:USER@DOMAIN". + * dst must fit at least MAX_URI_SIZE+1 characters! */ +static inline int sipuri_cat(char* dst, const str* user, const str* domain) { + if ((4 + user->len + 1 + domain->len) > MAX_URI_SIZE) { + LM_ERR("entity URI too long, maximum=%d\n", MAX_URI_SIZE); + return -1; + } + memcpy(dst, "sip:", 4); + memcpy(dst + 4, user->s, user->len); + dst[user->len + 4] = '@'; + memcpy(dst + user->len + 5, domain->s, domain->len); + dst[user->len + 5 + domain->len] = '\0'; + return 0; +} str* dlginfo_agg_nbody(str* pres_user, str* pres_domain, str** body_array, int n, int off_index) { str* n_body= NULL; char pres_uri_char[MAX_URI_SIZE+1]; - if ((pres_user->len + pres_domain->len + 5) > MAX_URI_SIZE) { - LM_ERR("entity URI too long, maximum=%d\n", MAX_URI_SIZE); + if (sipuri_cat(pres_uri_char, pres_user, pres_domain) != 0) return NULL; - } - memcpy(pres_uri_char, "sip:", 4); - memcpy(pres_uri_char + 4, pres_user->s, pres_user->len); - pres_uri_char[pres_user->len + 4] = '@'; - memcpy(pres_uri_char + pres_user->len + 5, pres_domain->s, pres_domain->len); - pres_uri_char[pres_user->len + 5 + pres_domain->len] = '\0'; - LM_DBG("[pres_uri] %s (%d), [n]=%d\n", pres_uri_char, pres_user->len + 5 + pres_domain->len, n); @@ -117,7 +123,7 @@ str* agregate_xmls(str* pres_user, str* pres_domain, str** body_array, int n, in int winner_priority = -1, priority ; xmlNodePtr winner_dialog_node = NULL ; str *body= NULL; - char buf[MAX_URI_SIZE+1]; + char buf[MAX_URI_SIZE+1]; LM_DBG("[pres_user]=%.*s [pres_domain]= %.*s, [n]=%d\n", pres_user->len, pres_user->s, pres_domain->len, pres_domain->s, n); @@ -163,15 +169,8 @@ str* agregate_xmls(str* pres_user, str* pres_domain, str** body_array, int n, in /* LM_DBG("number of bodies in total [n]=%d, number of useful bodies [j]=%d\n", n, j ); */ /* create the new NOTIFY body */ - if ( (pres_user->len + pres_domain->len + 1) > MAX_URI_SIZE) { - LM_ERR("entity URI too long, maximum=%d\n", MAX_URI_SIZE); - return NULL; - } - memcpy(buf, "sip:", 4); - memcpy(buf+4, pres_user->s, pres_user->len); - buf[pres_user->len+4] = '@'; - memcpy(buf + pres_user->len + 5, pres_domain->s, pres_domain->len); - buf[pres_user->len + 5 + pres_domain->len]= '\0'; + if (sipuri_cat(buf, pres_user, pres_domain) != 0) + goto error; doc = xmlNewDoc(BAD_CAST "1.0"); if(doc==0) @@ -360,37 +359,21 @@ str* build_dialoginfo(str* pres_user, str* pres_domain) xmlNodePtr state_node = NULL; str *body= NULL; - str *pres_uri= NULL; + str pres_uri; char buf[MAX_URI_SIZE+1]; - if ( (pres_user->len + pres_domain->len + 1) > MAX_URI_SIZE) { - LM_ERR("entity URI too long, maximum=%d\n", MAX_URI_SIZE); + if (sipuri_cat(buf, pres_user, pres_domain) != 0) return NULL; - } - memcpy(buf, "sip:", 4); - memcpy(buf+4, pres_user->s, pres_user->len); - buf[pres_user->len+4] = '@'; - memcpy(buf + pres_user->len + 5, pres_domain->s, pres_domain->len); - buf[pres_user->len + 5 + pres_domain->len]= '\0'; - - pres_uri = (str*)pkg_malloc(sizeof(str)); - if(pres_uri == NULL) - { - LM_ERR("while allocating memory\n"); - return NULL; - } - memset(pres_uri, 0, sizeof(str)); - pres_uri->s = buf; - pres_uri->len = pres_user->len + 5 + pres_domain->len; + pres_uri.s = buf; + pres_uri.len = 4 + pres_user->len + 1 + pres_domain->len; + LM_DBG("[pres_uri] %.*s\n", pres_uri.len, pres_uri.s); - LM_DBG("[pres_uri] %.*s\n", pres_uri->len, pres_uri->s); - - if ( pres_contains_presence(pres_uri)<0 ) { + if (pres_contains_presence(&pres_uri) < 0) { LM_DBG("No record exists in hash_table\n"); goto error; } - /* create the Publish body */ + /* create the Publish body */ doc = xmlNewDoc(BAD_CAST "1.0"); if(doc==0) goto error; @@ -415,8 +398,10 @@ str* build_dialoginfo(str* pres_user, str* pres_domain) LM_ERR("while adding child [dialog]\n"); goto error; } + + /* reuse buf for user-part only */ memcpy(buf, pres_user->s, pres_user->len); - buf[pres_user->len]= '\0'; + buf[pres_user->len] = '\0'; xmlNewProp(dialog_node, BAD_CAST "id", BAD_CAST buf); @@ -444,10 +429,6 @@ str* build_dialoginfo(str* pres_user, str* pres_domain) xmlCleanupParser(); return body; error: - if ( pres_uri ) - { - pkg_free(pres_uri); - } if(body) { if(body->s)