From 59d7daa02844f0f10cbee1f7f9e2fecff7929bde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= Date: Thu, 7 Jan 2021 16:08:40 +0100 Subject: [PATCH 1/4] Invalid dereference libmy/argv.c:3212: var_deref_model: Passing null pointer "queue_list" to "do_list", which dereferences it libmy/argv.c:3204: var_deref_model: Passing null pointer "queue_list" to "do_list", which dereferences it. Workaround to possibility no arguments is received Usually at least one arg is always passed in argv - program name. Do not dereference null queue_list in unlikely case no parameter in argv. --- argv.c | 43 +++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/argv.c b/argv.c index 6c64906..24ab779 100644 --- a/argv.c +++ b/argv.c @@ -3193,29 +3193,36 @@ int argv_process_no_env(argv_t *args, const int arg_n, char **argv) /* allocate our argument queue */ queue_list = (argv_t **)malloc(sizeof(argv_t *) * total_arg_n); if (queue_list == NULL) { + if (env_vect_p != NULL) { + free(env_vect_p); + free(environ_p); + } return ERROR; } queue_head = 0; queue_tail = 0; - } + + /* do the env args before? */ + if (argv_process_env_b && (! argv_env_after_b) && env_vect_p != NULL) { + do_list(args, env_n, env_vect_p, queue_list, &queue_head, &queue_tail, + &okay_b); + free(env_vect_p); + free(environ_p); + env_vect_p = NULL; + } + + /* do the external args */ + if (arg_n > 0) + do_list(args, arg_n - 1, argv + 1, queue_list, &queue_head, &queue_tail, + &okay_b); - /* do the env args before? */ - if (argv_process_env_b && (! argv_env_after_b) && env_vect_p != NULL) { - do_list(args, env_n, env_vect_p, queue_list, &queue_head, &queue_tail, - &okay_b); - free(env_vect_p); - free(environ_p); - env_vect_p = NULL; + /* DO the env args after? */ + if (argv_process_env_b && argv_env_after_b && env_vect_p != NULL) + do_list(args, env_n, env_vect_p, queue_list, &queue_head, &queue_tail, + &okay_b); } - - /* do the external args */ - do_list(args, arg_n - 1, argv + 1, queue_list, &queue_head, &queue_tail, - &okay_b); - - /* DO the env args after? */ - if (argv_process_env_b && argv_env_after_b && env_vect_p != NULL) { - do_list(args, env_n, env_vect_p, queue_list, &queue_head, &queue_tail, - &okay_b); + + if (env_vect_p != NULL) { free(env_vect_p); free(environ_p); env_vect_p = NULL; @@ -3233,7 +3240,7 @@ int argv_process_no_env(argv_t *args, const int arg_n, char **argv) } /* if we allocated the space then free it */ - if (arg_n > 0) { + if (queue_list) { free(queue_list); } From ba8a754c741cf715d54d9b65ea3fe76b39fedcc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= Date: Fri, 8 Jan 2021 13:23:17 +0100 Subject: [PATCH 2/4] Fix deadcode and check return code 1. fstrm-0.6.0/libmy/argv.c:1782: addr_non_null: The address of an object "argv_types" is never null. 2. fstrm-0.6.0/libmy/argv.c:1782: assignment: Assigning: "type_p" = "argv_types". 3. fstrm-0.6.0/libmy/argv.c:1809: notnull: At condition "type_p == NULL", the value of "type_p" cannot be "NULL". 4. fstrm-0.6.0/libmy/argv.c:1809: dead_error_condition: The condition "type_p == NULL" cannot be true. 5. fstrm-0.6.0/libmy/argv.c:1810: dead_error_begin: Execution cannot reach this statement: "(void)fprintf(argv_error_st...". 40. fstrm-0.6.0/libmy/argv.c:2724: check_return: Calling "string_to_value" without checking return value (as is done elsewhere 6 out of 7 times). --- argv.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/argv.c b/argv.c index 24ab779..ad55430 100644 --- a/argv.c +++ b/argv.c @@ -1806,7 +1806,7 @@ static void display_variables(const argv_t *args) int entry_c, size = 0; /* find the type and the size for array */ - if (type_p == NULL) { + if (type_p->at_value == 0) { (void)fprintf(argv_error_stream, "%s: illegal variable type %d\n", __FILE__, val_type); continue; @@ -2721,7 +2721,9 @@ static void do_list(argv_t *grid, const int arg_c, char **argv, case ARGV_LONG: case ARGV_FLOAT: case ARGV_DOUBLE: - string_to_value(*arg_p, match_p->ar_variable, match_p->ar_type); + if (string_to_value(*arg_p, match_p->ar_variable, match_p->ar_type) != NOERROR) { + *okay_bp = ARGV_FALSE; + } char_c = len; /* we actually used it so we advance the queue tail position */ (*queue_tail_p)++; From b81c4c77f7e6ed1e33bf66f7835b608a9c5b1aca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= Date: Fri, 8 Jan 2021 17:43:03 +0100 Subject: [PATCH 3/4] Possible resource leak fix 34. fstrm-0.6.0/libmy/argv.c:2238: alloc_fn: Storage is returned from allocation function "realloc". 35. fstrm-0.6.0/libmy/argv.c:2238: var_assign: Assigning: "argv" = storage returned from "realloc(argv, 8UL * max)". 37. fstrm-0.6.0/libmy/argv.c:2254: var_assign: Assigning: "argv_p" = "argv". 47. fstrm-0.6.0/libmy/argv.c:2229: leaked_storage: Variable "argv_p" going out of scope leaks the storage it points to. 48. fstrm-0.6.0/libmy/argv.c:2229: leaked_storage: Variable "argv" going out of scope leaks the storage it points to. --- argv.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/argv.c b/argv.c index ad55430..e777b66 100644 --- a/argv.c +++ b/argv.c @@ -2226,7 +2226,7 @@ static void file_args(const char *path, argv_t *grid, *argv_p = string_copy(line); if (*argv_p == NULL) { *okay_bp = ARGV_FALSE; - return; + goto cleanup; } argv_p++; @@ -2257,7 +2257,8 @@ static void file_args(const char *path, argv_t *grid, /* now do the list */ do_list(grid, arg_c, argv, queue_list, queue_head_p, queue_tail_p, okay_bp); - + +cleanup: /* now free up the list */ for (argv_p = argv; argv_p < argv + arg_c; argv_p++) { free(*argv_p); From 692ed9c67e0250aba6891a62a9405f81a81a35a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= Date: Mon, 11 Jan 2021 12:25:27 +0100 Subject: [PATCH 4/4] Fix CLANG_WARNING libmy/argv.c:1352:7: warning[core.uninitialized.Assign]: The expression is an uninitialized value. The computed value will also be garbage (*(int *)var)++; ^~~~~~~~~~~~~ libmy/argv.c:1207:29: note: Assuming field 'at_value' is not equal to 0 for (type_p = argv_types; type_p->at_value != 0; type_p++) { ^~~~~~~~~~~~~~~~~~~~~ libmy/argv.c:1207:3: note: Loop condition is true. Entering loop body for (type_p = argv_types; type_p->at_value != 0; type_p++) { ^ libmy/argv.c:1208:9: note: Assuming 'val_type' is equal to field 'at_value' if (type_p->at_value == val_type) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ libmy/argv.c:1208:5: note: Taking true branch if (type_p->at_value == val_type) { ^ libmy/argv.c:1210:7: note: Execution continues on line 1214 break; ^ libmy/argv.c:1214:15: note: Field 'at_value' is not equal to 0 if (type_p->at_value == 0) { ^ libmy/argv.c:1214:3: note: Taking false branch if (type_p->at_value == 0) { ^ libmy/argv.c:1222:7: note: Assuming the condition is true if (type & ARGV_FLAG_ARRAY) { ^~~~~~~~~~~~~~~~~~~~~~ libmy/argv.c:1222:3: note: Taking true branch if (type & ARGV_FLAG_ARRAY) { ^ libmy/argv.c:1225:9: note: Assuming field 'aa_entry_n' is equal to 0 if (arr_p->aa_entry_n == 0) { ^~~~~~~~~~~~~~~~~~~~~~ libmy/argv.c:1225:5: note: Taking true branch if (arr_p->aa_entry_n == 0) { ^ libmy/argv.c:1226:35: note: Storing uninitialized value arr_p->aa_entries = (char *)malloc(ARRAY_INCR *size); ^~~~~~~~~~~~~~~~~~~~~~~~ libmy/argv.c:1234:9: note: Assuming field 'aa_entries' is not equal to NULL if (arr_p->aa_entries == NULL) { ^~~~~~~~~~~~~~~~~~~~~~~~~ libmy/argv.c:1234:5: note: Taking false branch if (arr_p->aa_entries == NULL) { ^ libmy/argv.c:1251:3: note: Control jumps to 'case 17:' at line 1349 switch (val_type) { ^ libmy/argv.c:1351:9: note: Assuming 'arg' is equal to NULL if (arg == NULL) { ^~~~~~~~~~~ libmy/argv.c:1351:5: note: Taking true branch if (arg == NULL) { ^ libmy/argv.c:1352:7: note: The expression is an uninitialized value. The computed value will also be garbage (*(int *)var)++; ^~~~~~~~~~~~~ --- argv.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/argv.c b/argv.c index e777b66..b234789 100644 --- a/argv.c +++ b/argv.c @@ -1223,12 +1223,15 @@ static int string_to_value(const char *arg, ARGV_PNT var, arr_p = (argv_array_t *)var; if (arr_p->aa_entry_n == 0) { - arr_p->aa_entries = (char *)malloc(ARRAY_INCR *size); + arr_p->aa_entries = (char *)calloc(ARRAY_INCR, size); } else if (arr_p->aa_entry_n % ARRAY_INCR == 0) { arr_p->aa_entries = (char *)realloc(arr_p->aa_entries, (arr_p->aa_entry_n + ARRAY_INCR) * size); + if (arr_p->aa_entries != NULL) + memset((char *)(arr_p->aa_entries) + arr_p->aa_entry_n * size, 0, + ARRAY_INCR*size); } if (arr_p->aa_entries == NULL) {