pass: parser: Fix various defects including nonterminated string cases

This patch fixes the following code-level defects according to static
program analysis result:

1. PROC_USE.VULNERABLE: Use of vulnerable function 'strcpy'. For better
security, using strncpy is recommended instead of strcpy.
2. UNREACHABLE_CODE: The statement in the source code might be
unreachable during program execution.
3. DEREF_AFTER_NULL.EX: After having been compared to NULL value, some
pointers are dereferenced.
4. NONTERMINATED_STRING: Copying from string to a buffer without null
termination by calling function 'strncpy'.
5. TAINTED_INT.LOOP.MIGHT: Integer value obtained from untrusted source
without checking its higher bound is used as a loop bound.
6. TAINTED_INT/TAINTED_INT.MIGHT: Integer value obtained from untrusted
source without checking its higher bound is used in a trusted operation
by calling function 'calloc'.

Change-Id: Ic24ac658af88c109a22664c7c03eb2c43e8e23c4
Signed-off-by: Wook Song <wook16.song@samsung.com>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

diff --git a/src/pass/pass-parser.c b/src/pass/pass-parser.c
index e953c884dd8dee3ec63dea828076a252c5d267de..206f4289955731bf74535ba1060d9b426467a74c 100644 (file)
--- a/src/pass/pass-parser.c
+++ b/src/pass/pass-parser.c
@@ -28,6 +28,7 @@
 #include "hal/hal.h"
 
 #define BUFF_MAX                       255
+#define MAX_NUM                                255
 
 static enum pass_state is_supported(char *value)
 {
@@ -52,7 +53,7 @@ static int pass_parse_scenario(struct parse_result *result, void *user_data,
        struct pass_policy *policy = user_data;
        struct pass_scenario_policy *scenario = &policy->scenario;
 
-       if (!policy && !scenario && !result)
+       if (!policy || !result)
                return 0;
 
        if (!result->section || !result->name || !result->value)
@@ -66,7 +67,15 @@ static int pass_parse_scenario(struct parse_result *result, void *user_data,
                                return -EINVAL;
 
                } else if (MATCH(result->name, "pass_num_scenarios")) {
-                       scenario->num_scenarios = atoi(result->value);
+                       unsigned int num_scenarios;
+
+                       num_scenarios = atoi(result->value);
+                       if (num_scenarios > MAX_NUM) {
+                               _E("cannot parse %s\n", result->name);
+                               return -EINVAL;
+                       }
+
+                       scenario->num_scenarios = num_scenarios;
 
                        if (scenario->num_scenarios > 0 && !scenario->list) {
                                scenario->list = calloc(scenario->num_scenarios,
@@ -90,7 +99,8 @@ static int pass_parse_scenario(struct parse_result *result, void *user_data,
 
        /* Parse 'Scenario' section */
        if (MATCH(result->name, "name")) {
-               strcpy(scenario->list[index].name, result->value);
+               snprintf(scenario->list[index].name, PASS_NAME_LEN, "%s",
+                               result->value);
        } else if (MATCH(result->name, "support")) {
                scenario->list[index].state = is_supported(result->value);
                if (scenario->list[index].state < 0)
@@ -183,9 +193,15 @@ static int pass_parse_core(struct parse_result *result, void *user_data)
                policy->state = atoi(result->value);
        else if (MATCH(result->name, "pass_gov_type"))
                policy->gov_type = atoi(result->value);
-       else if (MATCH(result->name, "pass_num_levels"))
-               policy->num_levels = atoi(result->value);
-       else if (MATCH(result->name, "pass_min_level"))
+       else if (MATCH(result->name, "pass_num_levels")) {
+               unsigned int num_levels = atoi(result->value);
+
+               if (num_levels > MAX_NUM) {
+                       _E("cannot parse %s\n", result->name);
+                       return -EINVAL;
+               }
+               policy->num_levels = num_levels;
+       } else if (MATCH(result->name, "pass_min_level"))
                policy->min_level = atoi(result->value);
        else if (MATCH(result->name, "pass_max_level"))
                policy->max_level = atoi(result->value);
@@ -248,7 +264,7 @@ static int pass_load_config(struct parse_result *result, void *user_data)
 
        /* Parsing 'Level' section to get pass-table */
        for (level = 0; level < policy->num_levels; level++) {
-               ret = sprintf(section_name, "Level%d", level);
+               ret = snprintf(section_name, BUFF_MAX, "Level%d", level);
 
                if (MATCH(result->section, section_name)) {
                        ret = pass_parse_level(result, user_data, level);
@@ -274,7 +290,7 @@ static int pass_load_config(struct parse_result *result, void *user_data)
 
        /* Parsing 'Scenario' section */
        for (index = 0; index < scenario->num_scenarios; index++) {
-               ret = sprintf(section_name, "Scenario%d", index);
+               ret = snprintf(section_name, BUFF_MAX, "Scenario%d", index);
 
                if (MATCH(result->section, section_name)) {
                        ret = pass_parse_scenario(result, user_data, index);
@@ -463,13 +479,13 @@ static int  pass_parse_resource_data(struct parse_result *result,
                conf_data->res_type = atoi(result->value);
        } else if (MATCH(result->name, "pass_res_name")) {
                int len = strlen(result->value);
-               strncpy(conf_data->res_name, result->value, len);
+               snprintf(conf_data->res_name, len + 1, "%s", result->value);
        } else if (MATCH(result->name, "pass_path_conf_file")) {
                int len = strlen(result->value);
-               strncpy(conf_data->path_conf_file, result->value, len);
+               snprintf(conf_data->path_conf_file, len + 1, "%s", result->value);
        } else if (MATCH(result->name, "pass_path_load_table")) {
                int len = strlen(result->value);
-               strncpy(conf_data->path_load_table, result->value, len);
+               snprintf(conf_data->path_load_table, len + 1, "%s", result->value);
        } else if (MATCH(result->name, "pass_first_cpu")) {
                int pass_res_type = conf_data->res_type;
 
@@ -547,7 +563,7 @@ static int pass_resource_config(struct parse_result *result, void *user_data)
        if (MATCH(result->section, "PassResource")) {
                if (MATCH(result->name, "pass_compatible")) {
                        len = strlen(result->value);
-                       strncpy(compatible, result->value, len);
+                       snprintf(compatible, len + 1, "%s", result->value);
 
                        len = strlen(path_compatible);
                        if (len > 0) {
@@ -559,7 +575,7 @@ static int pass_resource_config(struct parse_result *result, void *user_data)
                        }
                } else if (MATCH(result->name, "pass_path_compatible")) {
                        len = strlen(result->value);
-                       strncpy(path_compatible, result->value, len);
+                       snprintf(path_compatible, len + 1, "%s", result->value);
 
                        len =strlen(compatible);
                        if (len > 0) {
@@ -570,7 +586,14 @@ static int pass_resource_config(struct parse_result *result, void *user_data)
                                is_compatible = true;
                        }
                } else if (MATCH(result->name, "pass_num_resources")) {
-                       pass->num_resources = atoi(result->value);
+                       unsigned int num_resources = atoi(result->value);
+
+                       if ((num_resources > MAX_NUM) ||
+                                       (num_resources < 1)) {
+                               _E("cannot parse %s\n", result->name);
+                               return -EINVAL;
+                       }
+                       pass->num_resources = num_resources;
 
                        pass->res = calloc(pass->num_resources,
                                        sizeof(struct pass_resource));
@@ -586,7 +609,7 @@ static int pass_resource_config(struct parse_result *result, void *user_data)
 
        /* Parsing 'PassResource[X]' section */
        for (id = 0; id < pass->num_resources; id++) {
-               ret = sprintf(section_name, "PassResource%d", id);
+               ret = snprintf(section_name, BUFF_MAX, "PassResource%d", id);
 
                if (MATCH(section_name, result->section)) {
                        if (!is_compatible) {