From c537cb8151e9436d88a9410e02ef11a093008ff0 Mon Sep 17 00:00:00 2001 From: Dusan Klinec Date: Sat, 5 Dec 2015 14:33:51 +0100 Subject: [PATCH 01/15] fixing coverity found defects - invalid memory access / memory corruptions --- flags.c | 2 +- ip_addr.h | 2 +- modules/acc/acc.c | 1 + modules/call_center/call_center.c | 2 +- modules/dialog/dlg_tophiding.c | 2 +- modules/mi_datagram/datagram_fnc.c | 2 +- modules/mi_xmlrpc/xr_parser.c | 1 + modules/presence/presentity.c | 2 +- modules/tm/t_reply.c | 4 ++-- 9 files changed, 10 insertions(+), 8 deletions(-) diff --git a/flags.c b/flags.c index a1c7830becd..94fccf85a6d 100644 --- a/flags.c +++ b/flags.c @@ -163,7 +163,7 @@ int get_flag_id_by_name(int flag_type, char *flag_name) return -1; } - if (flag_type < 0 || flag_type > FLAG_LIST_COUNT) { + if (flag_type < 0 || flag_type >= FLAG_LIST_COUNT) { LM_ERR("Invalid flag list: %d\n", flag_type); return -2; } diff --git a/ip_addr.h b/ip_addr.h index e7c84df8fe8..33086218f20 100644 --- a/ip_addr.h +++ b/ip_addr.h @@ -546,7 +546,7 @@ static inline struct hostent* ip_addr2he(str* name, struct ip_addr* ip) p_aliases[0]=0; /* no aliases*/ p_addr[1]=0; /* only one address*/ p_addr[0]=address; - len = (name->len < 256) ? name->len : 256; + len = (name->len < 255) ? name->len : 255; strncpy(hostname, name->s, len); hostname[len] = 0; if (ip->len>16) return 0; diff --git a/modules/acc/acc.c b/modules/acc/acc.c index 88582da952b..a4fcb70b18a 100644 --- a/modules/acc/acc.c +++ b/modules/acc/acc.c @@ -1488,6 +1488,7 @@ int acc_evi_request( struct sip_msg *rq, struct sip_msg *rpl) } ret = 1; end: + if (backup_idx!=-1) /* can be -1, jumped to end: label*/ evi_params[backup_idx]->next = evi_params[backup_idx + 1]; return ret; diff --git a/modules/call_center/call_center.c b/modules/call_center/call_center.c index 7e7c1ac841a..a0f38e50bda 100755 --- a/modules/call_center/call_center.c +++ b/modules/call_center/call_center.c @@ -669,7 +669,7 @@ static inline str* build_displayname(str *prefix, struct to_body *fh) l --; n = prefix->len; - if (n>l) n = l; + if (n>=l) n = l; memcpy( p, prefix->s , n); p += n; diff --git a/modules/dialog/dlg_tophiding.c b/modules/dialog/dlg_tophiding.c index 4b75681b391..b40bd9bb03a 100644 --- a/modules/dialog/dlg_tophiding.c +++ b/modules/dialog/dlg_tophiding.c @@ -132,7 +132,7 @@ int dlg_replace_contact(struct sip_msg* msg, struct dlg_cell* dlg) goto error; } - memcpy(prefix,"flags & DLG_FLAG_TOPH_KEEP_USER && ct_username_len > 0) { memcpy(prefix+5,ct_username,ct_username_len); prefix[prefix_len-1] = '@'; diff --git a/modules/mi_datagram/datagram_fnc.c b/modules/mi_datagram/datagram_fnc.c index 6846952b2cd..8b774539a6f 100644 --- a/modules/mi_datagram/datagram_fnc.c +++ b/modules/mi_datagram/datagram_fnc.c @@ -462,7 +462,7 @@ void mi_datagram_server(int rx_sock, int tx_sock) continue; } - LM_DBG("mi_buf is %s and we have received %i bytes\n",mi_buf, ret); + LM_DBG("mi_buf is %.*s and we have received %i bytes\n", ret, mi_buf, ret); /*mi_buff is not null terminated */ dtgram.start = mi_buf; dtgram.len = ret; dtgram.current = dtgram.start; diff --git a/modules/mi_xmlrpc/xr_parser.c b/modules/mi_xmlrpc/xr_parser.c index 3c62c0e8a45..0d77b2a78a6 100644 --- a/modules/mi_xmlrpc/xr_parser.c +++ b/modules/mi_xmlrpc/xr_parser.c @@ -218,6 +218,7 @@ struct mi_root * xr_parse_tree( xmlrpc_env * env, xmlrpc_value * paramArray ) { goto error; } free(byteStringValue); + byteStringValue = NULL; #endif diff --git a/modules/presence/presentity.c b/modules/presence/presentity.c index d45e5721601..0d479505074 100644 --- a/modules/presence/presentity.c +++ b/modules/presence/presentity.c @@ -106,7 +106,7 @@ int publ_send200ok(struct sip_msg *msg, int lexpire, str etag) LM_ERR("unsuccessful sprintf\n"); goto error; } - if(hdr_append.len > buf_len) + if(hdr_append.len >= buf_len) { LM_ERR("buffer size overflown\n"); goto error; diff --git a/modules/tm/t_reply.c b/modules/tm/t_reply.c index 8eb58f35a8e..ed4d8666a1e 100644 --- a/modules/tm/t_reply.c +++ b/modules/tm/t_reply.c @@ -876,7 +876,7 @@ static inline int t_pick_branch( struct cell *t, int *res_code, int *do_cancel) if ( t->uac[b].last_received<200 ) { if (t->uac[b].br_flags & minor_branch_flag) { *do_cancel = 1; - continue; + continue; /* if last branch, lowest_b remains -1 */ } return -2; } @@ -888,7 +888,7 @@ static inline int t_pick_branch( struct cell *t, int *res_code, int *do_cancel) } } /* find lowest branch */ LM_DBG("picked branch %d, code %d (prio=%d)\n", - lowest_b,t->uac[lowest_b].last_received,lowest_s); + lowest_b,lowest_b==-1 ? -1 : t->uac[lowest_b].last_received,lowest_s); *res_code=lowest_s; return lowest_b; From e09f09ae89aff31393b04e40b4f82632fd4f50ff Mon Sep 17 00:00:00 2001 From: Dusan Klinec Date: Sat, 5 Dec 2015 14:34:41 +0100 Subject: [PATCH 02/15] fixing coverity found defects - resource leakage --- daemonize.c | 3 ++- modules/db_text/dbt_file.c | 2 ++ modules/mi_fifo/fifo_fnc.c | 11 +++++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/daemonize.c b/daemonize.c index dd98399ee0b..793349dfb4d 100644 --- a/daemonize.c +++ b/daemonize.c @@ -213,7 +213,7 @@ void clean_write_pipeend(void) */ int daemonize(char* name, int * own_pgid) { - FILE *pid_stream; + FILE *pid_stream = NULL; pid_t pid; int r, p,rc; int pid_items; @@ -336,6 +336,7 @@ int daemonize(char* name, int * own_pgid) if (r<=0) { LM_ERR("unable to write pgid to file %s: %s\n", pid_file, strerror(errno)); + fclose(pid_stream); goto error; } fclose(pid_stream); diff --git a/modules/db_text/dbt_file.c b/modules/db_text/dbt_file.c index 66f64d26bc5..6fca7d0bf16 100644 --- a/modules/db_text/dbt_file.c +++ b/modules/db_text/dbt_file.c @@ -499,6 +499,8 @@ dbt_table_p dbt_load_file(const str *tbn, const str *dbn) clean: /// ????? FILL IT IN - incomplete row/column // memory leak?!?! with last incomplete row + if(fin) + fclose(fin); LM_DBG("error at row=%d col=%d c=%c\n", crow+1, ccol+1, c); if(dtp) dbt_table_free(dtp); diff --git a/modules/mi_fifo/fifo_fnc.c b/modules/mi_fifo/fifo_fnc.c index 5b44b9f90a4..d9edbac7f2a 100644 --- a/modules/mi_fifo/fifo_fnc.c +++ b/modules/mi_fifo/fifo_fnc.c @@ -100,15 +100,23 @@ FILE* mi_init_fifo_server(char *fifo_name, int mi_fifo_mode, /* make sure the read fifo will not close */ mi_fifo_write=open( fifo_name, O_WRONLY|O_NONBLOCK, 0); if (mi_fifo_write<0) { + fclose(fifo_stream); + close(mi_fifo_read); LM_ERR("fifo_write did not open: %s\n", strerror(errno)); return 0; } /* set read fifo blocking mode */ if ((opt=fcntl(mi_fifo_read, F_GETFL))==-1){ + fclose(fifo_stream); + close(mi_fifo_read); + close(mi_fifo_write); LM_ERR("fcntl(F_GETFL) failed: %s [%d]\n", strerror(errno), errno); return 0; } if (fcntl(mi_fifo_read, F_SETFL, opt & (~O_NONBLOCK))==-1){ + fclose(fifo_stream); + close(mi_fifo_read); + close(mi_fifo_write); LM_ERR("cntl(F_SETFL) failed: %s [%d]\n", strerror(errno), errno); return 0; } @@ -117,6 +125,9 @@ FILE* mi_init_fifo_server(char *fifo_name, int mi_fifo_mode, mi_buf = pkg_malloc(MAX_MI_FIFO_BUFFER); reply_fifo_s = pkg_malloc(MAX_MI_FILENAME); if ( mi_buf==NULL|| reply_fifo_s==NULL) { + fclose(fifo_stream); + close(mi_fifo_read); + close(mi_fifo_write); LM_ERR("no more private memory\n"); return 0; } From b22844e0a1d4d54d0f8f836837f8447154799011 Mon Sep 17 00:00:00 2001 From: Dusan Klinec Date: Sat, 5 Dec 2015 14:36:17 +0100 Subject: [PATCH 03/15] fixing coverity found defects - processing initialized variables, control flow, invalid expressions --- modules/auth/challenge.c | 2 +- modules/drouting/drouting.c | 2 +- modules/pua/send_subscribe.c | 4 ++-- modules/uac/uac.c | 2 +- modules/usrloc/ucontact.c | 2 +- proxy.c | 2 +- rw_locking.h | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/modules/auth/challenge.c b/modules/auth/challenge.c index 153ebd6b405..4898bf93ebb 100644 --- a/modules/auth/challenge.c +++ b/modules/auth/challenge.c @@ -147,7 +147,7 @@ static inline int challenge(struct sip_msg* _msg, gparam_p _realm, int _qop, int _code, char* _message, char* _challenge_msg) { int auth_hf_len; - struct hdr_field* h; + struct hdr_field* h = NULL; auth_body_t* cred = 0; char *auth_hf; int ret; diff --git a/modules/drouting/drouting.c b/modules/drouting/drouting.c index 55ea02b7fc4..1dbcecb8414 100644 --- a/modules/drouting/drouting.c +++ b/modules/drouting/drouting.c @@ -1138,7 +1138,7 @@ static int use_next_gw(struct sip_msg* msg, int_str val; pv_value_t pv_val; str ruri; - int ok; + int ok=0; pgw_t * dst; struct socket_info *sock; diff --git a/modules/pua/send_subscribe.c b/modules/pua/send_subscribe.c index 0989a11e6ee..986a7187292 100644 --- a/modules/pua/send_subscribe.c +++ b/modules/pua/send_subscribe.c @@ -666,7 +666,7 @@ ua_pres_t* subscribe_cbparam(subs_info_t* subs, int ua_flag) hentity->flag= subs->source_flag; hentity->event= subs->event; - hentity->ua_flag= hentity->ua_flag; + hentity->ua_flag= ua_flag; hentity->cb_param= subs->cb_param; return hentity; @@ -771,7 +771,7 @@ ua_pres_t* subs_cbparam_indlg(ua_pres_t* subs, int expires, int ua_flag) hentity->flag= subs->flag; hentity->event= subs->event; - hentity->ua_flag= hentity->ua_flag; + hentity->ua_flag= ua_flag; hentity->cb_param= subs->cb_param; hentity->hash_index = subs->hash_index; hentity->local_index = subs->local_index; diff --git a/modules/uac/uac.c b/modules/uac/uac.c index a5bb9886897..f40dd7cd271 100644 --- a/modules/uac/uac.c +++ b/modules/uac/uac.c @@ -363,7 +363,7 @@ static int w_replace_from(struct sip_msg* msg, char* p1, char* p2) str uri_s; str dsp_s; str *uri; - str *dsp; + str *dsp = NULL; if (p2==NULL) { p2 = p1; diff --git a/modules/usrloc/ucontact.c b/modules/usrloc/ucontact.c index b96bd4bf964..e93cfbfa50f 100644 --- a/modules/usrloc/ucontact.c +++ b/modules/usrloc/ucontact.c @@ -58,7 +58,7 @@ */ static int compute_next_hop(ucontact_t *contact) { - str uri; + str uri = {0,0}; struct sip_uri puri; if (contact->path.s && contact->path.len > 0) { diff --git a/proxy.c b/proxy.c index b7a767c42e4..0d5fadba8ff 100644 --- a/proxy.c +++ b/proxy.c @@ -78,7 +78,7 @@ int hostent_shm_cpy(struct hostent *dst, struct hostent* src) p += src->h_length; } - dst->h_addr = dst->h_addr_list[0]; + dst->h_addr = src->h_addr_list[0]; dst->h_addrtype = src->h_addrtype; dst->h_length = src->h_length; return 0; diff --git a/rw_locking.h b/rw_locking.h index c42a8b8c531..389b121ee91 100644 --- a/rw_locking.h +++ b/rw_locking.h @@ -29,7 +29,7 @@ inline static rw_lock_t * lock_init_rw(void) return new_lock; error: - if (new_lock->lock) + if (new_lock!=NULL && new_lock->lock) lock_dealloc(new_lock->lock); if (new_lock) shm_free(new_lock); From aeedb7d94439b774d35009899b04d2c6060437d0 Mon Sep 17 00:00:00 2001 From: Dusan Klinec Date: Sat, 5 Dec 2015 15:01:24 +0100 Subject: [PATCH 04/15] fixing coverity found defects - logical fix in ul callback check type, null dereference --- modules/dispatcher/dispatcher.c | 2 +- modules/usrloc/ul_callback.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/dispatcher/dispatcher.c b/modules/dispatcher/dispatcher.c index c4512ece889..6a77171ec7a 100644 --- a/modules/dispatcher/dispatcher.c +++ b/modules/dispatcher/dispatcher.c @@ -494,7 +494,7 @@ static void destroy(void) #define CHECK_INVALID_PARAM(param) do{ \ str_trim_spaces_lr(param); \ - if ((param).s[0] == ',' || (param).s[(param).len-1]==',') { \ + if ((param).s == NULL || (param).len == 0 || (param).s[0] == ',' || (param).s[(param).len-1]==',') { \ LM_ERR("Empty slot in param [%.*s]\n", (param).len, (param).s); \ return -1; \ } \ diff --git a/modules/usrloc/ul_callback.h b/modules/usrloc/ul_callback.h index 1f50b441fb9..904b0be4903 100644 --- a/modules/usrloc/ul_callback.h +++ b/modules/usrloc/ul_callback.h @@ -65,7 +65,7 @@ extern struct ulcb_head_list* ulcb_list; #define exists_ulcb_type(_types_) \ - ( (ulcb_list->reg_types)|(_types_) ) + ( (ulcb_list->reg_types)&(_types_) ) int init_ulcb_list(); From c004967f7c272e5b9043f5f3e330827f0682a009 Mon Sep 17 00:00:00 2001 From: Dusan Klinec Date: Sat, 5 Dec 2015 14:35:23 +0100 Subject: [PATCH 05/15] fixing coverity found defects - null dereference, break missing --- db/db.c | 13 ++++++------- evi/event_interface.c | 2 +- main.c | 1 + modules/alias_db/alookup.c | 2 +- modules/usrloc/dlist.c | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/db/db.c b/db/db.c index 64068dd7989..2f1d2eb0c66 100644 --- a/db/db.c +++ b/db/db.c @@ -254,18 +254,17 @@ int db_bind_mod(const str* mod, db_func_t* mydbf) */ db_con_t* db_do_init(const str* url, void* (*new_connection)()) { - struct db_id* id; - void* con; - db_con_t* res; - - int con_size = sizeof(db_con_t) + sizeof(void *) + url->len; - id = 0; - res = 0; + struct db_id* id = NULL; + void* con = NULL; + db_con_t* res = NULL; + int con_size = 0; if (!url || !url->s || !new_connection) { LM_ERR("invalid parameter value\n"); return 0; } + + con_size = sizeof(db_con_t) + sizeof(void *) + url->len; if (url->len > MAX_URL_LENGTH) { LM_ERR("SQL URL too long\n"); diff --git a/evi/event_interface.c b/evi/event_interface.c index ad197833c57..53af639690a 100644 --- a/evi/event_interface.c +++ b/evi/event_interface.c @@ -509,7 +509,7 @@ struct mi_root * mi_events_list(struct mi_root *cmd_tree, void *param) static int evi_print_subscriber(struct mi_node *rpl, evi_subs_p subs) { - evi_reply_sock *sock = subs->reply_sock; + evi_reply_sock *sock = subs != NULL ? subs->reply_sock : NULL; struct mi_node *node = NULL; str socket; diff --git a/main.c b/main.c index b183bde4109..7f654aaf7df 100644 --- a/main.c +++ b/main.c @@ -1195,6 +1195,7 @@ int main(int argc, char** argv) break; case 'R': received_dns|=DO_REV_DNS; + break; case 'd': #ifdef CHANGEABLE_DEBUG_LEVEL (*debug)++; diff --git a/modules/alias_db/alookup.c b/modules/alias_db/alookup.c index 1ada47ed477..8e72a091487 100644 --- a/modules/alias_db/alookup.c +++ b/modules/alias_db/alookup.c @@ -121,7 +121,7 @@ static int alias_db_query(struct sip_msg* _msg, char* _table, goto err_server; } - if (RES_ROW_N(db_res)<=0 || RES_ROWS(db_res)[0].values[0].nul != 0) { + if (db_res == NULL || RES_ROW_N(db_res)<=0 || RES_ROWS(db_res)[0].values[0].nul != 0) { LM_DBG("no alias found for R-URI\n"); if (db_res!=NULL && adbf.free_result(db_handle, db_res) < 0) LM_DBG("failed to freeing result of query\n"); diff --git a/modules/usrloc/dlist.c b/modules/usrloc/dlist.c index e3756e31b60..23b568cb670 100644 --- a/modules/usrloc/dlist.c +++ b/modules/usrloc/dlist.c @@ -197,7 +197,7 @@ static int get_all_db_ucontacts(void *buf, int len, unsigned int flags, row = RES_ROWS(res) + i; val = ROW_VALUES(row) + 3; /* cflags */ flag_list.s = (char *)VAL_STRING(val); - flag_list.len = strlen(flag_list.s); + flag_list.len = (val->nul || !flag_list.s) ? 0 : strlen(flag_list.s); LM_DBG("contact cflags: '%.*s'\n", flag_list.len, flag_list.s); From b8e2318ba58e56484294913e21a74a7cd79adf40 Mon Sep 17 00:00:00 2001 From: Dusan Klinec Date: Sat, 5 Dec 2015 15:18:55 +0100 Subject: [PATCH 06/15] fixing coverity found defects - null dereference, invalid e164 number check --- modules/rtpproxy/rtpproxy.c | 2 +- modules/uri/checks.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/rtpproxy/rtpproxy.c b/modules/rtpproxy/rtpproxy.c index 4a9d7c0c6c3..444263a3c73 100644 --- a/modules/rtpproxy/rtpproxy.c +++ b/modules/rtpproxy/rtpproxy.c @@ -2788,7 +2788,7 @@ static int move_bavp2dlg(struct sip_msg *msg, struct dlg_cell *dlg, str *rval1, } not_moved: - LM_DBG("nothing moved - message type %d\n", msg->first_line.type); + LM_DBG("nothing moved - message type %d\n", !msg ? -1 : msg->first_line.type); if (rval1) rval1->len = 0; if (rval2) rval2->len = 0; if (setid) *setid = DEFAULT_RTPP_SET_ID; diff --git a/modules/uri/checks.c b/modules/uri/checks.c index bb76da55b2e..7fe57430c08 100644 --- a/modules/uri/checks.c +++ b/modules/uri/checks.c @@ -353,7 +353,7 @@ static inline int e164_check(str* _user) if ((_user->len > 2) && (_user->len < 17) && ((_user->s)[0] == '+')) { for (i = 1; i <= _user->len; i++) { c = (_user->s)[i]; - if (c < '0' && c > '9') return -1; + if (c < '0' || c > '9') return -1; } return 1; } From ef3d3de7d0607126fd8f149aa4c19ca3e05bb7a9 Mon Sep 17 00:00:00 2001 From: Dusan Klinec Date: Sun, 6 Dec 2015 11:22:09 +0100 Subject: [PATCH 07/15] fixing coverity found defects - null dereference in dispatcher --- modules/dispatcher/dispatcher.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/dispatcher/dispatcher.c b/modules/dispatcher/dispatcher.c index 6a77171ec7a..5c0e2394386 100644 --- a/modules/dispatcher/dispatcher.c +++ b/modules/dispatcher/dispatcher.c @@ -493,8 +493,12 @@ static void destroy(void) }while(0) #define CHECK_INVALID_PARAM(param) do{ \ + if ((param).s == NULL || (param).len == 0) { \ + LM_ERR("Empty slot in param [%.*s]\n", (param).len, (param).s); \ + return -1; \ + } \ str_trim_spaces_lr(param); \ - if ((param).s == NULL || (param).len == 0 || (param).s[0] == ',' || (param).s[(param).len-1]==',') { \ + if ((param).len == 0 || (param).s[0] == ',' || (param).s[(param).len-1]==',') { \ LM_ERR("Empty slot in param [%.*s]\n", (param).len, (param).s); \ return -1; \ } \ From 92a7361eda2c6cd6e9ad637dab187cf87fefa987 Mon Sep 17 00:00:00 2001 From: Dusan Klinec Date: Sun, 6 Dec 2015 09:40:25 +0100 Subject: [PATCH 08/15] fixing coverity found defects - copying into fixed size buffer --- modules/ldap/iniparser.c | 9 ++++++--- modules/siptrace/siptrace.c | 10 +++++++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/modules/ldap/iniparser.c b/modules/ldap/iniparser.c index 25fca14aa8a..208afdff02c 100644 --- a/modules/ldap/iniparser.c +++ b/modules/ldap/iniparser.c @@ -548,6 +548,7 @@ output file pointers. /* iniparser.c.c following */ #define ASCIILINESZ 1024 +#define LONGKEYBUFF 2*ASCIILINESZ+1 #define INI_INVALID_KEY ((char*)-1) /* Private: add an entry to the dictionary */ @@ -557,13 +558,15 @@ output file pointers. char * key, char * val) { - char longkey[2*ASCIILINESZ+1]; + char longkey[LONGKEYBUFF]; /* Make a key as section:keyword */ if (key!=NULL) { - sprintf(longkey, "%s:%s", sec, key); + snprintf(longkey, LONGKEYBUFF, "%s:%s", sec, key); } else { - strcpy(longkey, sec); + size_t len = strlen(sec); + longkey[LONGKEYBUFF-1]=0; + strncpy(longkey, sec, len >= LONGKEYBUFF ? LONGKEYBUFF-1 : len); } /* Add (key,val) to dictionary */ diff --git a/modules/siptrace/siptrace.c b/modules/siptrace/siptrace.c index 0bdfc09ca92..b7855bb398f 100644 --- a/modules/siptrace/siptrace.c +++ b/modules/siptrace/siptrace.c @@ -1146,7 +1146,7 @@ static void trace_onreply_in(struct cell* t, int type, struct tmcb_params *ps) struct sip_msg* req; int_str avp_value; struct usr_avp *avp; - char statusbuf[8]; + char statusbuf[INT2STR_MAX_LEN]; int len; if(t==NULL || t->uas.request==0 || ps==NULL) @@ -1203,7 +1203,9 @@ static void trace_onreply_in(struct cell* t, int type, struct tmcb_params *ps) db_vals[2].val.str_val.s = t->method.s; db_vals[2].val.str_val.len = t->method.len; - strcpy(statusbuf, int2str(ps->code, &len)); + char * str_code = int2str(ps->code, &len); + statusbuf[INT2STR_MAX_LEN-1]=0; + strncpy(statusbuf, str_code, len >= INT2STR_MAX_LEN ? INT2STR_MAX_LEN-1 : len); db_vals[3].val.str_val.s = statusbuf; db_vals[3].val.str_val.len = len; @@ -1452,7 +1454,9 @@ static void trace_sl_onreply_out( unsigned int types, struct sip_msg* req, msg->rcv.dst_port, msg->rcv.proto); } - strcpy(statusbuf, int2str(sl_param->code, &len)); + char * str_code = int2str(sl_param->code, &len); + statusbuf[INT2STR_MAX_LEN-1]=0; + strncpy(statusbuf, str_code, len >= INT2STR_MAX_LEN ? INT2STR_MAX_LEN-1 : len); db_vals[3].val.str_val.s = statusbuf; db_vals[3].val.str_val.len = len; From d471234733de299c88f1036ba17a2406b8ae7a89 Mon Sep 17 00:00:00 2001 From: Dusan Klinec Date: Sun, 6 Dec 2015 09:42:17 +0100 Subject: [PATCH 09/15] fixing coverity found defects - copying into fixed size buffer without check, db_http --- modules/db_http/http_dbase.c | 76 ++++++++++++++++++++++++------------ 1 file changed, 50 insertions(+), 26 deletions(-) diff --git a/modules/db_http/http_dbase.c b/modules/db_http/http_dbase.c index 16ec5263476..f84f08efb3b 100644 --- a/modules/db_http/http_dbase.c +++ b/modules/db_http/http_dbase.c @@ -976,20 +976,20 @@ str url_encode(str s) db_con_t* db_http_init(const str* url) { - +#define DB_HTTP_BUFF_SIZE 1024 char* path; - char port [20]; - char user_pass[1024]; - char modified_url[1024]; + char user_pass[DB_HTTP_BUFF_SIZE]; + char modified_url[DB_HTTP_BUFF_SIZE]; str tmp; + int off, ret; - db_con_t * ans; - http_conn_t * curl; + db_con_t * ans = NULL; + http_conn_t * curl = NULL; int i; struct db_id * id; - memset(modified_url,0,1024); + memset(modified_url,0,DB_HTTP_BUFF_SIZE); memcpy(modified_url,url->s,url->len); strcat(modified_url,"/x"); @@ -999,7 +999,7 @@ db_con_t* db_http_init(const str* url) user_pass[0] = 0; - path = (char*)pkg_malloc(1024); + path = (char*)pkg_malloc(DB_HTTP_BUFF_SIZE); if( path == NULL ) { @@ -1007,13 +1007,14 @@ db_con_t* db_http_init(const str* url) return NULL; } - memset(path,0,1024); + memset(path,0,DB_HTTP_BUFF_SIZE); id = new_db_id( &tmp ); if( id == NULL) { + pkg_free(path); LM_ERR("Incorrect db_url\n"); return NULL; } @@ -1022,9 +1023,8 @@ db_con_t* db_http_init(const str* url) if( id->username && id->password) { - strcat(user_pass,id->username); - strcat(user_pass,":"); - strcat(user_pass,id->password); + ret = snprintf(user_pass, DB_HTTP_BUFF_SIZE, "%s:%s", id->username, id->password); + if (ret < 0 || ret >= DB_HTTP_BUFF_SIZE) goto error; } @@ -1033,6 +1033,7 @@ db_con_t* db_http_init(const str* url) if( curl == NULL ) { + pkg_free(path); LM_ERR("Out of memory\n"); return NULL; } @@ -1050,26 +1051,37 @@ db_con_t* db_http_init(const str* url) curl_easy_setopt(curl->handle,CURLOPT_TIMEOUT_MS,db_http_timeout); #endif - strcat(path,"http"); - if ( use_ssl ) - strcat(path,"s"); - strcat(path,"://"); + ret = snprintf(path, DB_HTTP_BUFF_SIZE, "http"); + if (ret < 0 || ret >= DB_HTTP_BUFF_SIZE) goto error; + off = ret; + if (use_ssl) { + ret = snprintf(path + off, DB_HTTP_BUFF_SIZE - off, "s"); + if (ret < 0 || ret >= (DB_HTTP_BUFF_SIZE - off)) goto error; + off += ret; + } - strcat(path,id->host); - if( id->port ) - { - strcat(path,":"); - sprintf(port,"%d",id->port); - strcat(path,port); + ret = snprintf(path + off, DB_HTTP_BUFF_SIZE - off, "://%s", id->host); + if (ret < 0 || ret >= (DB_HTTP_BUFF_SIZE - off)) goto error; + off += ret; + + if (id->port) { + ret = snprintf(path + off, DB_HTTP_BUFF_SIZE - off, ":%d", id->port); + if (ret < 0 || ret >= (DB_HTTP_BUFF_SIZE - off)) goto error; + off += ret; } - strcat(path,"/"); + + ret = snprintf(path + off, DB_HTTP_BUFF_SIZE - off, "/"); + if (ret < 0 || ret >= (DB_HTTP_BUFF_SIZE - off)) goto error; + off += ret; if( strlen(id->database) > 2 ) { id->database[strlen(id->database)-2] = 0; - strcat(path,id->database); - strcat(path,"/"); + + ret = snprintf(path + off, DB_HTTP_BUFF_SIZE - off, "%s/", id->database); + if (ret < 0 || ret >= (DB_HTTP_BUFF_SIZE - off)) goto error; + off += ret; } curl->start.s = path; @@ -1080,6 +1092,10 @@ db_con_t* db_http_init(const str* url) if( ans == NULL ) { + pkg_free(path); + curl_easy_cleanup(curl->handle); + pkg_free(curl); + LM_ERR("Out of memory\n"); return NULL; } @@ -1101,7 +1117,15 @@ db_con_t* db_http_init(const str* url) next_state[ ESC ][ (int) quote_delim ] = IN; return ans; - +error: + if (path) + pkg_free(path); + if (curl) { + curl_easy_cleanup(curl->handle); + pkg_free(curl); + } + LM_CRIT("Initialization error\n"); + return NULL; } From b578dda307f24e56f456e5b02c0c7591ed6746a4 Mon Sep 17 00:00:00 2001 From: Dusan Klinec Date: Sun, 6 Dec 2015 09:59:31 +0100 Subject: [PATCH 10/15] fixing coverity found defects - null dereference --- blacklists.c | 1 + modules/drouting/drouting.c | 6 +++--- modules/sipcapture/sipcapture.c | 2 +- modules/siptrace/siptrace.c | 2 +- resolve.h | 4 ++++ statistics.c | 2 +- 6 files changed, 11 insertions(+), 6 deletions(-) diff --git a/blacklists.c b/blacklists.c index b337581f0d2..b339d825c6a 100644 --- a/blacklists.c +++ b/blacklists.c @@ -420,6 +420,7 @@ static inline void rm_dups(struct bl_head *head, (r->proto==q->proto) && (ip_class_compare(&r->ip_net, &q->ip_net)==1) && ((!r->body.s && !q->body.s) || ((r->body.len==q->body.len) && + r->body.s!=NULL && q->body.s!=NULL && !strncmp(r->body.s,q->body.s,q->body.len)) ) ) { break; diff --git a/modules/drouting/drouting.c b/modules/drouting/drouting.c index 1dbcecb8414..a78d02dd875 100644 --- a/modules/drouting/drouting.c +++ b/modules/drouting/drouting.c @@ -1045,7 +1045,7 @@ static inline str* build_ruri(struct sip_uri *uri, int strip, str *pri, } memcpy(p, uri->user.s+strip, uri->user.len-strip); p += uri->user.len-strip; - if (uri->passwd.len) { + if (uri->passwd.s && uri->passwd.len) { *(p++)=':'; memcpy(p, uri->passwd.s, uri->passwd.len); p += uri->passwd.len; @@ -1053,12 +1053,12 @@ static inline str* build_ruri(struct sip_uri *uri, int strip, str *pri, *(p++)='@'; memcpy(p, hostport->s, hostport->len); p += hostport->len; - if (uri->params.len) { + if (uri->params.s && uri->params.len) { *(p++)=';'; memcpy(p, uri->params.s, uri->params.len); p += uri->params.len; } - if (uri->headers.len) { + if (uri->headers.s && uri->headers.len) { *(p++)='?'; memcpy(p, uri->headers.s, uri->headers.len); p += uri->headers.len; diff --git a/modules/sipcapture/sipcapture.c b/modules/sipcapture/sipcapture.c index 2e52d92d172..e247e5986e0 100644 --- a/modules/sipcapture/sipcapture.c +++ b/modules/sipcapture/sipcapture.c @@ -491,7 +491,7 @@ static int mod_init(void) { return -1; } - if(promisc_on && raw_interface.len) { + if(promisc_on && raw_interface.s && raw_interface.len) { memset(&ifr, 0, sizeof(ifr)); memcpy(ifr.ifr_name, raw_interface.s, raw_interface.len); diff --git a/modules/siptrace/siptrace.c b/modules/siptrace/siptrace.c index b7855bb398f..c8b10c4bac2 100644 --- a/modules/siptrace/siptrace.c +++ b/modules/siptrace/siptrace.c @@ -1390,7 +1390,7 @@ static void trace_sl_onreply_out( unsigned int types, struct sip_msg* req, struct usr_avp *avp; struct ip_addr to_ip; int len; - char statusbuf[5]; + char statusbuf[INT2STR_MAX_LEN]; if(req==NULL || sl_param==NULL) { diff --git a/resolve.h b/resolve.h index 1a9e10d40a8..3b75bdccee7 100644 --- a/resolve.h +++ b/resolve.h @@ -204,6 +204,7 @@ static inline struct ip_addr* str2ip(str* st) static struct ip_addr ip; unsigned char *s; + if (st == NULL || st->s == NULL) goto error_null; s=(unsigned char*)st->s; /*init*/ @@ -251,6 +252,9 @@ static inline struct ip_addr* str2ip(str* st) ip.len=4; return &ip; +error_null: + LM_DBG("Null pointer detected\n"); + return NULL; error_dots: LM_DBG("too %s dots in [%.*s]\n", (i>3)?"many":"few", st->len, st->s); diff --git a/statistics.c b/statistics.c index 50a096b3313..7fe6588fcce 100644 --- a/statistics.c +++ b/statistics.c @@ -605,7 +605,7 @@ int register_stat2( char *module, char *name, stat_var **pvar, else shm_free(stat); error: - if ( (flags&STAT_IS_FUNC)==0 ) + if ( (flags&STAT_IS_FUNC)==0 && pvar!=NULL) *pvar = 0; return -1; From 906bb9172b4ce0ab0e7bf4eca7da58211aa8ac5b Mon Sep 17 00:00:00 2001 From: Dusan Klinec Date: Sun, 6 Dec 2015 10:38:58 +0100 Subject: [PATCH 11/15] fixing coverity found defects - null dereference --- cachedb/cachedb_id.c | 10 +++++----- db/db_query.c | 9 ++++++--- modules/cachedb_local/cachedb_local.c | 8 ++++---- modules/cachedb_sql/cachedb_sql.c | 4 ++-- modules/db_postgres/dbase.c | 5 +++-- modules/dialog/dlg_profile.c | 4 ++-- modules/httpd/httpd_proc.c | 2 +- modules/load_balancer/lb_db.c | 4 ++-- modules/permissions/hash.c | 4 ++++ modules/presence/notify.c | 2 +- modules/registrar/lookup.c | 2 +- parser/sdp/sdp.c | 2 +- 12 files changed, 32 insertions(+), 24 deletions(-) diff --git a/cachedb/cachedb_id.c b/cachedb/cachedb_id.c index 5fd47ba182c..a0d1e0900cd 100644 --- a/cachedb/cachedb_id.c +++ b/cachedb/cachedb_id.c @@ -267,11 +267,11 @@ static int parse_cachedb_url(struct cachedb_id* id, const str* url) return 0; err: - if (id->scheme) pkg_free(id->scheme); - if (id->username) pkg_free(id->username); - if (id->password) pkg_free(id->password); - if (id->host) pkg_free(id->host); - if (id->database) pkg_free(id->database); + if (id && id->scheme) pkg_free(id->scheme); + if (id && id->username) pkg_free(id->username); + if (id && id->password) pkg_free(id->password); + if (id && id->host) pkg_free(id->host); + if (id && id->database) pkg_free(id->database); if (prev_token) pkg_free(prev_token); return -1; } diff --git a/db/db_query.c b/db/db_query.c index 869199e9a4f..0bca04c81d3 100644 --- a/db/db_query.c +++ b/db/db_query.c @@ -120,7 +120,8 @@ int db_do_query(const db_con_t* _h, const db_key_t* _k, const db_op_t* _op, err_exit: if (_r) *_r = NULL; - CON_OR_RESET(_h); + if (_h) + CON_OR_RESET(_h); return -1; } @@ -343,7 +344,8 @@ int db_do_delete(const db_con_t* _h, const db_key_t* _k, const db_op_t* _o, error: LM_ERR("error while preparing delete operation\n"); err_exit: - CON_OR_RESET(_h); + if (_h) + CON_OR_RESET(_h); return -1; } @@ -394,7 +396,8 @@ int db_do_update(const db_con_t* _h, const db_key_t* _k, const db_op_t* _o, error: LM_ERR("error while preparing update operation\n"); err_exit: - CON_OR_RESET(_h); + if(_h) + CON_OR_RESET(_h); return -1; } diff --git a/modules/cachedb_local/cachedb_local.c b/modules/cachedb_local/cachedb_local.c index 8707f92197a..b075d7ac213 100644 --- a/modules/cachedb_local/cachedb_local.c +++ b/modules/cachedb_local/cachedb_local.c @@ -203,13 +203,13 @@ lcache_con* lcache_new_connection(struct cachedb_id* id) { lcache_con *con; - if (id->flags != CACHEDB_ID_NO_URL) { - LM_ERR("bogus url for local cachedb\n"); + if (id == NULL) { + LM_ERR("null db_id\n"); return 0; } - if (id == NULL) { - LM_ERR("null db_id\n"); + if (id->flags != CACHEDB_ID_NO_URL) { + LM_ERR("bogus url for local cachedb\n"); return 0; } diff --git a/modules/cachedb_sql/cachedb_sql.c b/modules/cachedb_sql/cachedb_sql.c index 14fc09d6380..3d35c1c8dff 100644 --- a/modules/cachedb_sql/cachedb_sql.c +++ b/modules/cachedb_sql/cachedb_sql.c @@ -206,7 +206,7 @@ static int dbcache_get(cachedb_con *con, str* attr, str* res) return -1; } - if (RES_ROW_N(db_res) <= 0 || RES_ROWS(db_res)[0].values[0].nul != 0) { + if (db_res == NULL || RES_ROW_N(db_res) <= 0 || RES_ROWS(db_res)[0].values[0].nul != 0) { LM_DBG("no value found for keyI\n"); if (db_res != NULL && cdb_dbf.free_result(cdb_db_handle, db_res) < 0) LM_DBG("failed to free result of query\n"); @@ -373,7 +373,7 @@ static int dbcache_fetch_counter(cachedb_con *con,str *attr,int *ret_val) return -1; } - if (RES_ROW_N(db_res) <= 0 || RES_ROWS(db_res)[0].values[0].nul != 0) { + if (db_res == NULL || RES_ROW_N(db_res) <= 0 || RES_ROWS(db_res)[0].values[0].nul != 0) { LM_DBG("no value found for keyI\n"); if (db_res != NULL && cdb_dbf.free_result(cdb_db_handle, db_res) < 0) LM_DBG("failed to free result of query\n"); diff --git a/modules/db_postgres/dbase.c b/modules/db_postgres/dbase.c index f862a1120b4..6f411d22d54 100644 --- a/modules/db_postgres/dbase.c +++ b/modules/db_postgres/dbase.c @@ -270,9 +270,10 @@ int db_postgres_fetch_result(const db_con_t* _con, db_res_t** _res, const int nr LM_ERR("%p - invalid query, execution aborted\n", _con); LM_ERR("%p - PQresultStatus(%s)\n",_con,PQresStatus(pqresult)); LM_ERR("%p: %s\n",_con,PQresultErrorMessage(CON_RESULT(_con))); - if (*_res) + if (*_res) { db_free_result(*_res); - *_res = 0; + *_res = 0; + } return -3; case PGRES_EMPTY_QUERY: diff --git a/modules/dialog/dlg_profile.c b/modules/dialog/dlg_profile.c index 42f84574799..a97284733f7 100644 --- a/modules/dialog/dlg_profile.c +++ b/modules/dialog/dlg_profile.c @@ -1130,8 +1130,8 @@ struct mi_root * mi_get_profile_values(struct mi_root *cmd_tree, void *param ) return rpl_tree; error: - - free_mi_tree(rpl_tree); + if (rpl_tree) + free_mi_tree(rpl_tree); return NULL; } diff --git a/modules/httpd/httpd_proc.c b/modules/httpd/httpd_proc.c index 75a6191e583..6e7980afd73 100644 --- a/modules/httpd/httpd_proc.c +++ b/modules/httpd/httpd_proc.c @@ -218,7 +218,7 @@ static int post_iterator (void *cls, pr = (struct post_request*)cls; if (pr==NULL) { LM_CRIT("corrupted data: null cls\n"); - pr->status = -1; return MHD_NO; + return MHD_NO; } if (off!=0) { diff --git a/modules/load_balancer/lb_db.c b/modules/load_balancer/lb_db.c index 2b6e4b33cda..8b32db89aed 100644 --- a/modules/load_balancer/lb_db.c +++ b/modules/load_balancer/lb_db.c @@ -116,7 +116,7 @@ int init_lb_db(const str *db_url, char *table) int lb_db_load_data( struct lb_data *data) { db_key_t columns[5]; - db_res_t* res; + db_res_t* res = NULL; db_row_t* row; int i, n; char *resource, *uri; @@ -151,7 +151,7 @@ int lb_db_load_data( struct lb_data *data) } } - if (RES_ROW_N(res) == 0) { + if (res == NULL || RES_ROW_N(res) == 0) { LM_WARN("table \"%.*s\" empty\n", lb_table_name.len,lb_table_name.s ); return 0; } diff --git a/modules/permissions/hash.c b/modules/permissions/hash.c index 347811bc4a7..aa7a39c6eed 100644 --- a/modules/permissions/hash.c +++ b/modules/permissions/hash.c @@ -230,6 +230,10 @@ int find_group_in_hash_table(struct address_list** table, struct address_list *node; str str_ip; + if (ip == NULL){ + return -1; + } + str_ip.len = ip->len; str_ip.s = (char*) ip->u.addr; diff --git a/modules/presence/notify.c b/modules/presence/notify.c index aae88601d1c..f27b39f6f6d 100644 --- a/modules/presence/notify.c +++ b/modules/presence/notify.c @@ -2129,7 +2129,7 @@ void p_tm_callback( struct cell *t, int type, struct tmcb_params *ps) ((c_back_param*)(*ps->param))->to_tag.s== NULL) { LM_DBG("message id not received\n"); - if(*ps->param !=NULL ) + if(ps->param!=NULL && *ps->param !=NULL ) free_cbparam((c_back_param*)(*ps->param)); return; } diff --git a/modules/registrar/lookup.c b/modules/registrar/lookup.c index 231638ac55f..6948a342a10 100644 --- a/modules/registrar/lookup.c +++ b/modules/registrar/lookup.c @@ -165,7 +165,7 @@ int lookup(struct sip_msg* _m, char* _t, char* _f, char* _s) it = ptr->next; while ( it ) { if (VALID_CONTACT(it,act_time)) { - if (it->instance.len-2 == sip_instance.len && + if (it->instance.len-2 == sip_instance.len && sip_instance.s && memcmp(it->instance.s+1,sip_instance.s,sip_instance.len) == 0) if (it->last_modified > ptr->last_modified) { /* same instance id, but newer modified -> expired GRUU, no match at all */ diff --git a/parser/sdp/sdp.c b/parser/sdp/sdp.c index 0b60de0e43b..8498d25ce06 100644 --- a/parser/sdp/sdp.c +++ b/parser/sdp/sdp.c @@ -825,7 +825,7 @@ void print_sdp_stream(sdp_stream_cell_t *stream, int log_level) void print_sdp_session(sdp_session_cell_t *session, int log_level) { - sdp_stream_cell_t *stream = session->streams; + sdp_stream_cell_t *stream = session==NULL ? NULL : session->streams; if (session==NULL) { LM_ERR("NULL session\n"); From 81aeba42ca04df7b3fc42ab2498717cc31227967 Mon Sep 17 00:00:00 2001 From: Dusan Klinec Date: Sun, 6 Dec 2015 10:39:18 +0100 Subject: [PATCH 12/15] fixing coverity found defects - null dereference & broken logic --- modules/db_postgres/val.c | 1 + parser/parse_uri.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/db_postgres/val.c b/modules/db_postgres/val.c index f561b78df92..0234f13fb3c 100644 --- a/modules/db_postgres/val.c +++ b/modules/db_postgres/val.c @@ -60,6 +60,7 @@ int db_postgres_str2val(const db_type_t _t, db_val_t* _v, const char* _s, const if (!_v) { LM_ERR("invalid parameter value\n"); + return -1; } if (!_s) { diff --git a/parser/parse_uri.c b/parser/parse_uri.c index 661f7146f60..276f6e04dce 100644 --- a/parser/parse_uri.c +++ b/parser/parse_uri.c @@ -1366,7 +1366,7 @@ int compare_uris(str *raw_uri_a,struct sip_uri* parsed_uri_a, char matched[URI_MAX_U_PARAMS]; int i,j; - if ( (!raw_uri_a && !parsed_uri_b) || (!raw_uri_b && !parsed_uri_b) ) + if ( (!raw_uri_a && !parsed_uri_a) || (!raw_uri_b && !parsed_uri_b) ) { LM_ERR("Provide either a raw or parsed form of a SIP URI\n"); return -1; From fd57609f5411e622dca593b729e1826269243c5b Mon Sep 17 00:00:00 2001 From: Dusan Klinec Date: Sun, 6 Dec 2015 10:39:41 +0100 Subject: [PATCH 13/15] fixing coverity found defects - memory corruption, null dereference --- modules/enum/enum.c | 6 ++++++ modules/permissions/rule.c | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/modules/enum/enum.c b/modules/enum/enum.c index 49028d15c4a..95f4a724be1 100644 --- a/modules/enum/enum.c +++ b/modules/enum/enum.c @@ -402,6 +402,12 @@ int is_from_user_enum_2(struct sip_msg* _msg, char* _suffix, char* _service) proto = PROTO_NONE; he = sip_resolvehost(&luri.host, &zp, &proto, (luri.type==SIPS_URI_T)?1:0 , 0); + if (he == NULL){ + LM_ERR("Resolving URI <%.*s> failed\n", + result.len, result.s); + free_rdata_list(head); /*clean up*/ + return -9; + } hostent2ip_addr(&addr, he, 0); diff --git a/modules/permissions/rule.c b/modules/permissions/rule.c index 597e5c3a95e..3e13b6674eb 100644 --- a/modules/permissions/rule.c +++ b/modules/permissions/rule.c @@ -130,6 +130,11 @@ expression *new_expression(char *str) return 0; } + if (strlen(str) > EXPRESSION_LENGTH){ + LM_ERR("expresion too long\n"); + pkg_free(e); + return 0; + } strcpy(e->value, str); e->reg_value = (regex_t*)pkg_malloc(sizeof(regex_t)); From 4594e82dd7e11b9139fe2950246b7ce61c78d0c9 Mon Sep 17 00:00:00 2001 From: Dusan Klinec Date: Sun, 6 Dec 2015 18:48:45 +0100 Subject: [PATCH 14/15] fixing coverity found defects - null dereference --- resolve.c | 2 +- resolve.h | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/resolve.c b/resolve.c index 231d28eecdf..4bebb53fcea 100644 --- a/resolve.c +++ b/resolve.c @@ -1825,7 +1825,7 @@ struct hostent* sip_resolvehost( str* name, unsigned short* port, } if ( dns_try_naptr==0 ) { - *proto = (is_sips)?PROTO_TLS:PROTO_UDP; + if (proto) *proto = (is_sips)?PROTO_TLS:PROTO_UDP; goto do_srv; } LM_DBG("no port, no proto -> do NAPTR lookup!\n"); diff --git a/resolve.h b/resolve.h index 3b75bdccee7..50cfff4e18b 100644 --- a/resolve.h +++ b/resolve.h @@ -284,6 +284,8 @@ static inline struct ip_addr* str2ip6(str* st) unsigned char* limit; unsigned char* s; + if (st == NULL || st->s == NULL) goto error_null; + /* init */ if ((st->len) && (st->s[0]=='[')){ /* skip over [ ] */ @@ -346,6 +348,9 @@ static inline struct ip_addr* str2ip6(str* st) addr_start[6], addr_start[7] ); */ return &ip; +error_null: + LM_DBG("Null pointer detected\n"); + return 0; error_too_many_colons: LM_DBG("too many colons in [%.*s]\n", st->len, st->s); From 92c69284aa1edf084ef91930f976627578d65323 Mon Sep 17 00:00:00 2001 From: Dusan Klinec Date: Thu, 14 Jan 2016 17:14:15 +0100 Subject: [PATCH 15/15] Fixing non-null terminated string bug, found by @liviuchircu --- modules/ldap/iniparser.c | 2 +- modules/siptrace/siptrace.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/ldap/iniparser.c b/modules/ldap/iniparser.c index 208afdff02c..887372da06e 100644 --- a/modules/ldap/iniparser.c +++ b/modules/ldap/iniparser.c @@ -565,8 +565,8 @@ output file pointers. snprintf(longkey, LONGKEYBUFF, "%s:%s", sec, key); } else { size_t len = strlen(sec); - longkey[LONGKEYBUFF-1]=0; strncpy(longkey, sec, len >= LONGKEYBUFF ? LONGKEYBUFF-1 : len); + longkey[LONGKEYBUFF-1]=0; } /* Add (key,val) to dictionary */ diff --git a/modules/siptrace/siptrace.c b/modules/siptrace/siptrace.c index c8b10c4bac2..3a3d79f0763 100644 --- a/modules/siptrace/siptrace.c +++ b/modules/siptrace/siptrace.c @@ -1204,8 +1204,8 @@ static void trace_onreply_in(struct cell* t, int type, struct tmcb_params *ps) db_vals[2].val.str_val.len = t->method.len; char * str_code = int2str(ps->code, &len); - statusbuf[INT2STR_MAX_LEN-1]=0; strncpy(statusbuf, str_code, len >= INT2STR_MAX_LEN ? INT2STR_MAX_LEN-1 : len); + statusbuf[INT2STR_MAX_LEN-1]=0; db_vals[3].val.str_val.s = statusbuf; db_vals[3].val.str_val.len = len; @@ -1455,8 +1455,8 @@ static void trace_sl_onreply_out( unsigned int types, struct sip_msg* req, } char * str_code = int2str(sl_param->code, &len); - statusbuf[INT2STR_MAX_LEN-1]=0; strncpy(statusbuf, str_code, len >= INT2STR_MAX_LEN ? INT2STR_MAX_LEN-1 : len); + statusbuf[INT2STR_MAX_LEN-1]=0; db_vals[3].val.str_val.s = statusbuf; db_vals[3].val.str_val.len = len;