Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix coverity issues in carrierroute module #1199

Merged
merged 7 commits into from
Aug 1, 2017
Merged

Fix coverity issues in carrierroute module #1199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3c86d68
carrierroute: avoid out-of-bounds access when resetting opts
Jul 31, 2017
7f603ec
carrierroute: check pointer before dereferencing it
Jul 31, 2017
2772f09
carrieroute: remove logically dead code
Jul 31, 2017
d8fdcb3
carrierroute: avoid comparing size_t types with negative values
Aug 1, 2017
3a9365f
carrierroute: check return value for fseek
Aug 1, 2017
dbafb5a
carrierroute: avoid truncating value returned by fgetc
Aug 1, 2017
535eb88
carrierroute: avoid ovverruning a fixed sized buffer
Aug 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
125 changes: 59 additions & 66 deletions src/modules/carrierroute/cr_config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,78 +48,71 @@ enum prefix_opt_ids { PO_MAX_TARGETS = 0, PO_MAX_IDS };
option_description target_options[TO_MAX_IDS];
option_description prefix_options[PO_MAX_IDS];

static void reset_opts(option_description * opts, int size){
static void reset_target_opts(){
int i;
if ( NULL == opts){
LM_ERR("Trying to init a NULL pointer location \n");
return;
}
for (i=0; i < size; i++){
memset(&(opts[i].value),'\0', sizeof(union opt_data));
opts[i].visited = 0;
opts[i].no_elems = 0;
if ( CFG_STR == opts[i].type ){
opts[i].value.string_data.s = opts[i].str_buf;
strcpy(opts[i].str_buf,"");
opts[i].value.string_data.len = 0;
for (i=0; i < TO_MAX_IDS; i++){
memset(&(target_options[i].value),'\0', sizeof(union opt_data));
target_options[i].visited = 0;
target_options[i].no_elems = 0;
if ( CFG_STR == target_options[i].type ){
target_options[i].value.string_data.s = target_options[i].str_buf;
strcpy(target_options[i].str_buf,"");
target_options[i].value.string_data.len = 0;
}
}

opts[TO_ID_STRIP ].value.int_data=0;
opts[TO_ID_PROB ].value.float_data=0;
opts[TO_ID_HASH_INDEX].value.int_data=0;
opts[TO_ID_STATUS ].value.int_data=0;
opts[TO_ID_BACKUP ].value.int_data=-1;

return;
target_options[TO_ID_STRIP ].value.int_data=0;
target_options[TO_ID_PROB ].value.float_data=0;
target_options[TO_ID_HASH_INDEX].value.int_data=0;
target_options[TO_ID_STATUS ].value.int_data=0;
target_options[TO_ID_BACKUP ].value.int_data=-1;
}

static int init_target_opts(option_description * opts){
if ( NULL == opts){
LM_DBG("Trying to init a NULL pointer location \n");
return -1;
}
memset(opts, '\0', sizeof(option_description) * TO_MAX_IDS);

strcpy((char*)(opts[TO_ID_COMMENT].name), "comment");
strcpy((char*)(opts[TO_ID_STRIP].name), "strip");
strcpy((char*)(opts[TO_ID_REWR_PREFIX].name),"rewrite_prefix");
strcpy((char*)(opts[TO_ID_PROB].name), "prob");
strcpy((char*)(opts[TO_ID_HASH_INDEX].name), "hash_index");
strcpy((char*)(opts[TO_ID_REWR_SUFFIX].name),"rewrite_suffix");
strcpy((char*)(opts[TO_ID_STATUS].name), "status");
strcpy((char*)(opts[TO_ID_BACKED_UP].name), "backed_up");
strcpy((char*)(opts[TO_ID_BACKUP].name), "backup");

opts[TO_ID_COMMENT ].type=CFG_STR;
opts[TO_ID_STRIP ].type=CFG_INT;
opts[TO_ID_REWR_PREFIX].type=CFG_STR;
opts[TO_ID_PROB ].type=CFG_FLOAT;
opts[TO_ID_HASH_INDEX ].type=CFG_INT;
opts[TO_ID_REWR_SUFFIX].type=CFG_STR;
opts[TO_ID_STATUS ].type=CFG_INT;
opts[TO_ID_BACKED_UP ].type=CFG_INT_LIST;
opts[TO_ID_BACKUP ].type=CFG_INT;

reset_opts(opts, TO_MAX_IDS);
static int init_target_opts(){

memset(target_options, '\0', sizeof(option_description) * TO_MAX_IDS);

strcpy((char*)(target_options[TO_ID_COMMENT].name), "comment");
strcpy((char*)(target_options[TO_ID_STRIP].name), "strip");
strcpy((char*)(target_options[TO_ID_REWR_PREFIX].name),"rewrite_prefix");
strcpy((char*)(target_options[TO_ID_PROB].name), "prob");
strcpy((char*)(target_options[TO_ID_HASH_INDEX].name), "hash_index");
strcpy((char*)(target_options[TO_ID_REWR_SUFFIX].name),"rewrite_suffix");
strcpy((char*)(target_options[TO_ID_STATUS].name), "status");
strcpy((char*)(target_options[TO_ID_BACKED_UP].name), "backed_up");
strcpy((char*)(target_options[TO_ID_BACKUP].name), "backup");

target_options[TO_ID_COMMENT ].type=CFG_STR;
target_options[TO_ID_STRIP ].type=CFG_INT;
target_options[TO_ID_REWR_PREFIX].type=CFG_STR;
target_options[TO_ID_PROB ].type=CFG_FLOAT;
target_options[TO_ID_HASH_INDEX ].type=CFG_INT;
target_options[TO_ID_REWR_SUFFIX].type=CFG_STR;
target_options[TO_ID_STATUS ].type=CFG_INT;
target_options[TO_ID_BACKED_UP ].type=CFG_INT_LIST;
target_options[TO_ID_BACKUP ].type=CFG_INT;

reset_target_opts();
return 0;
}

static int init_prefix_opts(option_description * opts){
if ( NULL == opts){
LM_DBG("Trying to init a NULL pointer location \n");
return -1;
}
memset(opts, '\0', sizeof(option_description) * PO_MAX_IDS);
strcpy((char*)(opts[PO_MAX_TARGETS].name), "max_targets");
opts[PO_MAX_TARGETS].type=CFG_INT;
opts[PO_MAX_TARGETS].value.int_data=-1;
static void reset_prefix_opts(){
prefix_options[PO_MAX_TARGETS].value.int_data=-1;
}

static int init_prefix_opts(){

memset(prefix_options, '\0', sizeof(option_description) * PO_MAX_IDS);
strcpy((char*)(prefix_options[PO_MAX_TARGETS].name), "max_targets");
prefix_options[PO_MAX_TARGETS].type=CFG_INT;
reset_prefix_opts();
return 0;
}

static int backup_config(void) {
FILE * from, * to;
char * backup_file, ch;
char * backup_file;
int ch;
LM_INFO("start configuration backup\n");
if((backup_file = pkg_malloc(strlen(config_file) + strlen (".bak") + 1)) == NULL){
PKG_MEM_ERROR;
Expand Down Expand Up @@ -258,22 +251,22 @@ int load_config(struct route_data_t * rd) {

/* Create and insert carrier data structure */
tmp_carrier_data = create_carrier_data(1, &rd->carrier_map[0].name, allocated_domain_num);
tmp_carrier_data->domain_num = 0;
tmp_carrier_data->id = 1;
tmp_carrier_data->name = &(rd->carrier_map[0].name);

if (tmp_carrier_data == NULL) {
LM_ERR("can't create new carrier\n");
goto errclose;
}
tmp_carrier_data->domain_num = 0;
tmp_carrier_data->id = 1;
tmp_carrier_data->name = &(rd->carrier_map[0].name);

if (add_carrier_data(rd, tmp_carrier_data) < 0) {
LM_ERR("couldn't add carrier data\n");
destroy_carrier_data(tmp_carrier_data);
goto errclose;
}

init_prefix_opts(prefix_options);
init_target_opts(target_options);
init_prefix_opts();
init_target_opts();

/* add all routes by parsing the route conf file */
/* while there are domain structures, get name and parse the structure*/
Expand Down Expand Up @@ -346,7 +339,7 @@ int load_config(struct route_data_t * rd) {
while ((ret_prefix = parse_struct_header(file, "prefix", &prefix_name))
== SUCCESSFUL_PARSING) {

reset_opts(prefix_options, PO_MAX_IDS);
reset_prefix_opts();
if (str_strcasecmp(&prefix_name, &CR_EMPTY_PREFIX) == 0) {
prefix_name.s[0] = '\0';
prefix_name.len = 0;
Expand All @@ -369,7 +362,7 @@ int load_config(struct route_data_t * rd) {
goto errclose;
}

reset_opts(target_options, TO_MAX_IDS);
reset_target_opts();
/* look for the target options: prob, hash index, status, etc*/
ret_target_opts = parse_options(file, target_options, TO_MAX_IDS, "}");
if ( SUCCESSFUL_PARSING == ret_target_opts ){
Expand Down
6 changes: 4 additions & 2 deletions src/modules/carrierroute/cr_db.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,15 +280,17 @@ int load_route_data_db(struct route_data_t * rd) {
return -1;
}

if((rd->carrier_num = load_carrier_map(rd)) <= 0){
if((ret = load_carrier_map(rd)) <= 0){
LM_ERR("error while retrieving carriers\n");
goto errout;
}
rd->carrier_num = (size_t)ret;

if((rd->domain_num = load_domain_map(rd)) <= 0){
if((ret = load_domain_map(rd)) <= 0){
LM_ERR("error while retrieving domains\n");
goto errout;
}
rd->domain_num = (size_t)ret;

if ((rd->carriers = shm_malloc(sizeof(struct carrier_data_t *) * rd->carrier_num)) == NULL) {
SHM_MEM_ERROR;
Expand Down
11 changes: 8 additions & 3 deletions src/modules/carrierroute/cr_fifo.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1074,14 +1074,19 @@ static int cr_rpc_dump_tree_recursor (rpc_t* rpc, void* ctx, void *gh,
char s[256];
char rbuf[1024];
char *p;
int i;
int i,len;
struct route_flags *rf;
struct route_rule *rr;
struct route_rule_p_list * rl;
double prob;

len=strlen(prefix);
if (len > 254) {
LM_ERR("prefix too large");
return -1;
}
strcpy (s, prefix);
p = s + strlen (s);
p = s + len;
p[1] = '\0';
for (i = 0; i < cr_match_mode; ++i) {
if (node->child[i] != NULL) {
Expand All @@ -1101,7 +1106,7 @@ static int cr_rpc_dump_tree_recursor (rpc_t* rpc, void* ctx, void *gh,
}
snprintf(rbuf, 1024,
"%10s: %0.3f %%, '%.*s': %s, '%i', '%.*s', '%.*s', '%.*s'",
strlen(prefix) > 0 ? prefix : "NULL", prob * 100,
len > 0 ? prefix : "NULL", prob * 100,
rr->host.len, rr->host.s,
(rr->status ? "ON" : "OFF"), rr->strip,
rr->local_prefix.len, rr->local_prefix.s,
Expand Down
14 changes: 2 additions & 12 deletions src/modules/carrierroute/cr_func.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,12 +673,7 @@ int cr_do_route(struct sip_msg * _msg, gparam_t *_carrier,
}

carrier_data=NULL;
if (carrier_id < 0) {
if (cfg_get(carrierroute, carrierroute_cfg, fallback_default)) {
LM_NOTICE("invalid tree id %i specified, using default tree\n", carrier_id);
carrier_data = get_carrier_data(rd, rd->default_carrier_id);
}
} else if (carrier_id == 0) {
if (carrier_id == 0) {
carrier_data = get_carrier_data(rd, rd->default_carrier_id);
} else {
carrier_data = get_carrier_data(rd, carrier_id);
Expand Down Expand Up @@ -889,12 +884,7 @@ int cr_load_next_domain(struct sip_msg * _msg, gparam_t *_carrier,
}

carrier_data=NULL;
if (carrier_id < 0) {
if (cfg_get(carrierroute, carrierroute_cfg, fallback_default)) {
LM_NOTICE("invalid tree id %i specified, using default tree\n", carrier_id);
carrier_data = get_carrier_data(rd, rd->default_carrier_id);
}
} else if (carrier_id == 0) {
if (carrier_id == 0) {
carrier_data = get_carrier_data(rd, rd->default_carrier_id);
} else {
carrier_data = get_carrier_data(rd, carrier_id);
Expand Down
13 changes: 9 additions & 4 deletions src/modules/carrierroute/parser_carrierroute.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,10 @@ int parse_options(FILE* file, option_description* opts, int no_options, char* en
/* parsing stops when end_str is reached: e.g: }, target */
if ( strncmp(data.s, end_str, strlen(end_str)) == 0){
LM_DBG("End of options list received \n");
fseek(file, -full_line_len, SEEK_CUR);
if (fseek(file, -full_line_len, SEEK_CUR) != 0){
LM_ERR("fseek failed: %s",strerror(errno));
break;
}
ret = SUCCESSFUL_PARSING;
break;
}
Expand Down Expand Up @@ -385,9 +388,11 @@ int parse_struct_header(FILE* file, char* expected_struct_type, str* struct_name
memcpy(struct_name->s, name, struct_name->len);
struct_name->s[struct_name->len]='\0';
}
else
fseek(file, -full_line_len, SEEK_CUR);

else{
if (fseek(file, -full_line_len, SEEK_CUR) != 0){
LM_ERR("fseek failed: %s",strerror(errno));
}
}
return ret;
}

Expand Down