From a135a8987625fbc4fce542b18b6aa798cff0faa2 Mon Sep 17 00:00:00 2001 From: Peter Howkins Date: Sat, 28 Apr 2018 02:51:10 +0100 Subject: [PATCH] dtappbuilder: Further coverity, resource leaks, copy intofixed size buffer and dereference before null checl --- cde/programs/dtappbuilder/src/ab/ab_utils.c | 3 +- cde/programs/dtappbuilder/src/ab/abobj_util.c | 2 + cde/programs/dtappbuilder/src/ab/brws.c | 4 +- cde/programs/dtappbuilder/src/ab/brws_utils.c | 12 ++++-- cde/programs/dtappbuilder/src/ab/cgen_utils.c | 2 + cde/programs/dtappbuilder/src/ab/proj.c | 4 +- cde/programs/dtappbuilder/src/ab/proj_stubs.c | 24 ++++++++---- cde/programs/dtappbuilder/src/ab/proj_utils.c | 7 ++-- .../dtappbuilder/src/abmf/instances.c | 6 ++- .../dtappbuilder/src/abmf/obj_names.c | 39 ++++++++++--------- .../dtappbuilder/src/abmf/ui_c_file.c | 4 +- .../dtappbuilder/src/libABil/bil_store.c | 2 +- .../dtappbuilder/src/libABobj/obj_list.c | 2 + .../dtappbuilder/src/libABobj/obj_utils.c | 9 +++-- .../dtappbuilder/src/libABobj/trav_safe.c | 1 + .../dtappbuilder/src/libAButil/istr.c | 2 + .../dtappbuilder/src/libAButil/strlist.c | 2 + .../dtappbuilder/src/libAButil/util_file.c | 1 + 18 files changed, 82 insertions(+), 44 deletions(-) diff --git a/cde/programs/dtappbuilder/src/ab/ab_utils.c b/cde/programs/dtappbuilder/src/ab/ab_utils.c index ea629b60..e8258f56 100644 --- a/cde/programs/dtappbuilder/src/ab/ab_utils.c +++ b/cde/programs/dtappbuilder/src/ab/ab_utils.c @@ -1085,12 +1085,13 @@ format_dir_name_for_user( ) { STRING home= getenv(home_env_var_name); - int home_name_len= strlen(home); + int home_name_len = 0; int i= 0; if (home != NULL) { char home_relative[MAXPATHLEN]; + home_name_len = strlen(home); *home_relative = 0; util_cvt_path_to_relative(ugly_dir, home, home_relative, MAXPATHLEN); diff --git a/cde/programs/dtappbuilder/src/ab/abobj_util.c b/cde/programs/dtappbuilder/src/ab/abobj_util.c index b34044b5..7f0824e0 100644 --- a/cde/programs/dtappbuilder/src/ab/abobj_util.c +++ b/cde/programs/dtappbuilder/src/ab/abobj_util.c @@ -811,6 +811,7 @@ abobj_update_proj_name( catgets(Dtb_project_catd, 10, 1, "Project Organizer")); strcat(new_title, " - "); strcat(new_title, proj_win_title); + util_free(proj_win_title); XtVaSetValues(XtParent(AB_proj_window), XmNtitle, new_title, NULL); @@ -873,6 +874,7 @@ abobj_update_palette_title( catgets(Dtb_project_catd, 10, 5, "Application Builder")); strcat(new_title, " - "); strcat(new_title, proj_win_title); + util_free(proj_win_title); if (SaveNeeded) { diff --git a/cde/programs/dtappbuilder/src/ab/brws.c b/cde/programs/dtappbuilder/src/ab/brws.c index cf7c7bbe..2d76274d 100644 --- a/cde/programs/dtappbuilder/src/ab/brws.c +++ b/cde/programs/dtappbuilder/src/ab/brws.c @@ -705,8 +705,10 @@ browser_rband( /* * Return if no selected nodes */ - if (num_selected == 0) + if (num_selected == 0) { + util_free(selected_nodes); return; + } new_sel.list = (ABObj *)util_malloc(num_selected * sizeof(ABObj)); new_sel.count = 0; diff --git a/cde/programs/dtappbuilder/src/ab/brws_utils.c b/cde/programs/dtappbuilder/src/ab/brws_utils.c index 07c41ffc..ed37a9ff 100644 --- a/cde/programs/dtappbuilder/src/ab/brws_utils.c +++ b/cde/programs/dtappbuilder/src/ab/brws_utils.c @@ -1420,8 +1420,10 @@ brwsP_collapse_selected( /* * Return if no selected nodes */ - if (num_selected == 0) + if (num_selected == 0) { + util_free(selected_nodes); return; + } for (i=0; i < num_selected; ++i) { @@ -1480,8 +1482,10 @@ brwsP_expand_selected( /* * Return if no selected nodes */ - if (num_selected == 0) + if (num_selected == 0) { + util_free(selected_nodes); return; + } for (i=0; i < num_selected; ++i) { @@ -1540,8 +1544,10 @@ brwsP_expand_collapsed( /* * Return if no collapsed nodes */ - if (num_collapsed == 0) + if (num_collapsed == 0) { + free(collapsed_nodes); return; + } for (i=0; i < num_collapsed; ++i) { diff --git a/cde/programs/dtappbuilder/src/ab/cgen_utils.c b/cde/programs/dtappbuilder/src/ab/cgen_utils.c index c87a9f1b..14de0e77 100644 --- a/cde/programs/dtappbuilder/src/ab/cgen_utils.c +++ b/cde/programs/dtappbuilder/src/ab/cgen_utils.c @@ -2711,12 +2711,14 @@ destroy_links_to_file(STRING fileName) strlist_add_str(doomedFiles, fileName, NULL); if (stat(fileName, &doomedFileInfo) != 0) { + util_free(doomedFiles); return ERR_OPEN; } dir = opendir("."); if (dir == NULL) { + util_free(doomedFiles); return ERR_INTERNAL; } diff --git a/cde/programs/dtappbuilder/src/ab/proj.c b/cde/programs/dtappbuilder/src/ab/proj.c index bc93f8ac..d8261fd3 100644 --- a/cde/programs/dtappbuilder/src/ab/proj.c +++ b/cde/programs/dtappbuilder/src/ab/proj.c @@ -1523,8 +1523,10 @@ project_rband( /* * Return if no selected nodes */ - if (num_selected == 0) + if (num_selected == 0) { + free(selected_nodes); return; + } /* * For each object enclosed in rubber band rectangle diff --git a/cde/programs/dtappbuilder/src/ab/proj_stubs.c b/cde/programs/dtappbuilder/src/ab/proj_stubs.c index 567a920b..77574b83 100644 --- a/cde/programs/dtappbuilder/src/ab/proj_stubs.c +++ b/cde/programs/dtappbuilder/src/ab/proj_stubs.c @@ -218,8 +218,10 @@ projP_save_mod_proc( vwr_get_cond(v->current_tree, &selected_nodes, &num_selected, select_fn); - if (num_selected == 0) + if (num_selected == 0) { + free(selected_nodes); return; + } obj = (AB_OBJ *)selected_nodes[0]->obj_data; @@ -292,8 +294,7 @@ projP_save_mod_proc( /* * Free up node list if it contained anything */ - if (selected_nodes) - free((char *)selected_nodes); + free(selected_nodes); /*** DTB_USER_CODE_END ^^^ Add C variables and code above ^^^ ***/ @@ -331,15 +332,16 @@ projP_save_as_mod_proc( */ vwr_get_cond(v->current_tree, &selected_nodes, &num_selected, select_fn); - if (num_selected == 0) + if (num_selected == 0) { + free(selected_nodes); return; + } obj = (AB_OBJ *)selected_nodes[0]->obj_data; projP_show_save_as_bil_chooser(AB_toplevel, obj); - if (selected_nodes) - free((char *)selected_nodes); + free((char *)selected_nodes); /*** DTB_USER_CODE_END ^^^ Add C variables and code above ^^^ ***/ @@ -517,8 +519,10 @@ projP_browse_proc( vwr_get_cond(v->current_tree, &selected_nodes, &num_selected, select_fn); - if (num_selected == 0) + if (num_selected == 0) { + free(selected_nodes); return; + } for (i = 0; i < num_selected; ++i) { @@ -777,12 +781,16 @@ projP_export_mod_proc( vwr_get_cond(v->current_tree, &selected_nodes, &num_selected, select_fn); - if (num_selected == 0) + if (num_selected == 0) { + free(selected_nodes); return; + } obj = (AB_OBJ *)selected_nodes[0]->obj_data; proj_show_export_bil_chooser(AB_proj_window, obj); + free(selected_nodes); + /*** DTB_USER_CODE_END ^^^ Add C code above ^^^ ***/ } diff --git a/cde/programs/dtappbuilder/src/ab/proj_utils.c b/cde/programs/dtappbuilder/src/ab/proj_utils.c index 688bb4cf..470f22a5 100644 --- a/cde/programs/dtappbuilder/src/ab/proj_utils.c +++ b/cde/programs/dtappbuilder/src/ab/proj_utils.c @@ -1695,9 +1695,9 @@ proj_save_exploded( { util_print_error(rc, new_filename); obj_set_name(project, old_name); - if (old_name != NULL) util_free(old_name); - if (old_file != NULL) util_free(old_file); - if (old_proj_dir != NULL) util_free(old_proj_dir); + util_free(old_name); + util_free(old_file); + util_free(old_proj_dir); return rc; } obj_set_file(project, new_filename); @@ -2391,6 +2391,7 @@ proj_set_menus( */ vwr_get_cond(proj_vwr->current_tree, &selected_nodes, &num_selected, select_fn); + free(selected_nodes); /* Unused variable */ switch (chooser_type) { diff --git a/cde/programs/dtappbuilder/src/abmf/instances.c b/cde/programs/dtappbuilder/src/abmf/instances.c index 4a5fc770..6f4bb55b 100644 --- a/cde/programs/dtappbuilder/src/abmf/instances.c +++ b/cde/programs/dtappbuilder/src/abmf/instances.c @@ -514,6 +514,8 @@ write_assign_local_vars_for_fchooser(GenCodeInfo genCodeInfo, ABObj obj) abmfP_pattern_xmstr_var_has_value(genCodeInfo) = TRUE; } } + + return 0; } static int @@ -692,7 +694,7 @@ abmfP_strip_item_name(char *item_name) static char new_name[MAX_NAME_SIZE]; char *p; - strcpy(new_name, item_name); + snprintf(new_name, sizeof(new_name), "%s", item_name); p = (char *) strrchr(new_name, '_'); if (p != NULL) *p = '\0'; @@ -2461,7 +2463,7 @@ abmfP_get_widget_parent_name(GenCodeInfo genCodeInfo, ABObj obj) } if (widgetParent != NULL) { - strcpy(parentName, + snprintf(parentName, sizeof(parentName), "%s", abmfP_get_c_name(genCodeInfo, widgetParent)); parentFound = TRUE; } diff --git a/cde/programs/dtappbuilder/src/abmf/obj_names.c b/cde/programs/dtappbuilder/src/abmf/obj_names.c index 0c867f85..6dec0722 100644 --- a/cde/programs/dtappbuilder/src/abmf/obj_names.c +++ b/cde/programs/dtappbuilder/src/abmf/obj_names.c @@ -147,14 +147,12 @@ abmfP_get_c_name_global(ABObj obj) } fieldName = abmfP_get_c_field_name(obj); - strcpy(name, structName); - strcat(name, "."); if (substructName != NULL) { - strcat(name, substructName); - strcat(name, "."); + snprintf(name, sizeof(name), "%s.%s.%s", structName, substructName, fieldName); + } else { + snprintf(name, sizeof(name), "%s.%s", structName, fieldName); } - strcat(name, fieldName); return name; } @@ -275,14 +273,16 @@ abmfP_get_c_name_in_inst(ABObj obj) } fieldName = abmfP_get_c_field_name(obj); - strcpy(name, abmfP_instance_ptr_var_name); - strcat(name, "->"); if (substructName != NULL) { - strcat(name, substructName); - strcat(name, "."); + snprintf(name, sizeof(name), "%s->%s.%s", + abmfP_instance_ptr_var_name, + substructName, fieldName); + } else { + snprintf(name, sizeof(name), "%s->%s", + abmfP_instance_ptr_var_name, + fieldName); } - strcat(name, fieldName); return name; } @@ -695,6 +695,7 @@ STRING abmfP_get_c_substruct_ptr_type_name(ABObj obj) { static char ptrTypeName[MAX_NAME_SIZE]; + char ptrTypeNameTmp[sizeof(ptrTypeName)]; STRING varName = NULL; ABObj module = NULL; @@ -722,11 +723,12 @@ abmfP_get_c_substruct_ptr_type_name(ABObj obj) return NULL; else { - strcpy(ptrTypeName, typePrefixString); - strcat(ptrTypeName, - abmfP_capitalize_first_char(obj_get_name(module))); - strcat(ptrTypeName, abmfP_capitalize_first_char(varName)); - strcat(ptrTypeName, "Items"); + /* Warning: Due to abmfP_capitalize_first_char() returning a pointer + * to static data this cannot be one snprintf() */ + snprintf(ptrTypeNameTmp, sizeof(ptrTypeNameTmp), "%s%s", + typePrefixString, abmfP_capitalize_first_char(obj_get_name(module))); + snprintf(ptrTypeName, sizeof(ptrTypeName), "%s%sItems", + ptrTypeNameTmp, abmfP_capitalize_first_char(varName)); cvt_ident_to_type(ptrTypeName); } @@ -1119,13 +1121,12 @@ ensure_unique_comp_field_names(ABObj obj) } { char newObjName[1024]; - *newObjName = 0; if (compRootName != NULL) { - strcat(newObjName, compRootName); + snprintf(newObjName, sizeof(newObjName), "%s_%s", compRootName, ext); + } else { + snprintf(newObjName, sizeof(newObjName), "_%s", ext); } - strcat(newObjName, "_"); - strcat(newObjName, ext); util_dprintf(2, "changing field name %s -> %s\n", util_strsafe(obj_get_name(compRoot)), newObjName); diff --git a/cde/programs/dtappbuilder/src/abmf/ui_c_file.c b/cde/programs/dtappbuilder/src/abmf/ui_c_file.c index 950d64fe..97f27552 100644 --- a/cde/programs/dtappbuilder/src/abmf/ui_c_file.c +++ b/cde/programs/dtappbuilder/src/abmf/ui_c_file.c @@ -1328,11 +1328,13 @@ abmfP_get_msg_action_list( ABObj action = NULL; ABObj fromObj = NULL; AB_TRAVERSAL trav; - StringList callback_funcs = strlist_create(); + StringList callback_funcs = NULL; if (!obj_is_message(msg_obj)) return NULL; + callback_funcs = strlist_create(); + module = obj_get_module(msg_obj); for (trav_open(&trav, module, AB_TRAV_ACTIONS); (action = trav_next(&trav)) != NULL; ) diff --git a/cde/programs/dtappbuilder/src/libABil/bil_store.c b/cde/programs/dtappbuilder/src/libABil/bil_store.c index 1e0c0b38..0f458d80 100644 --- a/cde/programs/dtappbuilder/src/libABil/bil_store.c +++ b/cde/programs/dtappbuilder/src/libABil/bil_store.c @@ -3008,7 +3008,7 @@ create_bil_file_list( first = 0; if (strcpy(bil_list, file) == NULL) { - if (bil_list) util_free(bil_list); + util_free(bil_list); return NULL; } } diff --git a/cde/programs/dtappbuilder/src/libABobj/obj_list.c b/cde/programs/dtappbuilder/src/libABobj/obj_list.c index 6f5eba30..e24bf100 100644 --- a/cde/programs/dtappbuilder/src/libABobj/obj_list.c +++ b/cde/programs/dtappbuilder/src/libABobj/obj_list.c @@ -566,6 +566,8 @@ objlistP_grow_array(ABObjList list, int sizeDiff) if ( (new_objs == NULL) || (user_datas_valid && (new_user_datas == NULL)) ) { + free(new_objs); + free(new_user_datas); return_value = ERR_NO_MEMORY; goto epilogue; } diff --git a/cde/programs/dtappbuilder/src/libABobj/obj_utils.c b/cde/programs/dtappbuilder/src/libABobj/obj_utils.c index 18150482..6eb1c522 100644 --- a/cde/programs/dtappbuilder/src/libABobj/obj_utils.c +++ b/cde/programs/dtappbuilder/src/libABobj/obj_utils.c @@ -1197,13 +1197,14 @@ obj_verify(ABObj obj) if (ok) { obj_str_ptr_name = istr_string(obj->name); - sprintf((STRING)obj_name, "(ABObj %#lx", (unsigned long) obj); if (obj_str_ptr_name != NULL) { - strcat((STRING)obj_name, " = "); - strcat((STRING)obj_name, obj_str_ptr_name); + snprintf((STRING)obj_name, sizeof(obj_name), + "(ABObj %#lx = %s)", (unsigned long) obj, obj_str_ptr_name); + } else { + snprintf((STRING)obj_name, sizeof(obj_name), + "(ABObj %#lx)", (unsigned long) obj); } - strcat((STRING)obj_name, ")"); if (!ok) { field_err("name"); diff --git a/cde/programs/dtappbuilder/src/libABobj/trav_safe.c b/cde/programs/dtappbuilder/src/libABobj/trav_safe.c index 7601605e..d56f96b6 100644 --- a/cde/programs/dtappbuilder/src/libABobj/trav_safe.c +++ b/cde/programs/dtappbuilder/src/libABobj/trav_safe.c @@ -102,6 +102,7 @@ travP_open_safe( /* don't call travP_close(trav) - keep the traversal open */ if (iRet < 0) { + util_free(objArray); return iRet; } diff --git a/cde/programs/dtappbuilder/src/libAButil/istr.c b/cde/programs/dtappbuilder/src/libAButil/istr.c index 6206ba1b..b06b50f4 100644 --- a/cde/programs/dtappbuilder/src/libAButil/istr.c +++ b/cde/programs/dtappbuilder/src/libAButil/istr.c @@ -446,6 +446,7 @@ istrP_create_alloced_impl( fprintf(stderr, "%s", catgets(UTIL_MESSAGE_CATD, UTIL_MESSAGE_SET, 2, "ISTR: error in allocating space for string\n") ); + free(new_bucket); return NULL; } new_bucket->values[0] = freelist[freecount-1]; @@ -625,6 +626,7 @@ istr_create_const( fprintf(stderr, "%s", catgets(UTIL_MESSAGE_CATD, UTIL_MESSAGE_SET, 2, "ISTR: error in allocating space for string\n") ); + free(new_bucket); return NULL; } new_bucket->values[0] = freelist[freecount-1]; diff --git a/cde/programs/dtappbuilder/src/libAButil/strlist.c b/cde/programs/dtappbuilder/src/libAButil/strlist.c index a402a324..67045725 100644 --- a/cde/programs/dtappbuilder/src/libAButil/strlist.c +++ b/cde/programs/dtappbuilder/src/libAButil/strlist.c @@ -641,6 +641,8 @@ strlistP_grow_array(StringList list, int sizeDiff) if ((new_strings == NULL) || (new_user_datas == NULL)) { return_value = -1; + free(new_strings); + free(new_user_datas); goto epilogue; } else diff --git a/cde/programs/dtappbuilder/src/libAButil/util_file.c b/cde/programs/dtappbuilder/src/libAButil/util_file.c index 52aebe8c..381b6c7c 100644 --- a/cde/programs/dtappbuilder/src/libAButil/util_file.c +++ b/cde/programs/dtappbuilder/src/libAButil/util_file.c @@ -563,6 +563,7 @@ util_derive_name_from_path( len = strlen(name) - (AB_EXT_LENGTH + 1); strncpy(objname, name, len); objname[len] = '\0'; + free(name); } else { -- 2.25.1