From 914f3bd32d606383f278e2e0897c1eda09d6fedb Mon Sep 17 00:00:00 2001 From: "pr.jung" Date: Mon, 21 Jan 2019 19:09:03 +0900 Subject: [PATCH] Fix svace issues - An integer underflow may occur due to arithmetic operation (unsigned subtraction) between values '0' and '1', where the first value comes from the expression 'len - strlen(value)' - An integer underflow may occur due to arithmetic operation (unsigned subtraction) between variable 'len' and value { [0, 4294967295] } of 'strlen(value)', when 'len' is in range { [1, 2147483647] } - Possible integer overflow: right operand is tainted. An integer overflow may occur due to arithmetic operation (addition) between value { [-2147483648, 2147483647] } and variable 'duration', where the value comes from 'conf->pattern_duration' and when 'duration' is tainted { [-2147483648, 2147483647] } - Simplifying loop in insert_raw_data_format Change-Id: I70d7e24b8473d22207ce0197c04eac97c155eadf Signed-off-by: pr.jung --- src/haptic/standard-vibcore.c | 190 ++++++++++++++++++++---------------------- 1 file changed, 89 insertions(+), 101 deletions(-) diff --git a/src/haptic/standard-vibcore.c b/src/haptic/standard-vibcore.c index 2a1cf3e..eb2403d 100644 --- a/src/haptic/standard-vibcore.c +++ b/src/haptic/standard-vibcore.c @@ -40,25 +40,22 @@ /* 1,A_W or A,A_W or 250D250W250D750W - 85,10000,205,0, - 90,0,205,0, - 105,10000,205,0, + 85,10000, + 90,0, + 105,10000, 0,end */ struct vibration_config { char *pattern; /* pattern name */ char *standard; /* assigned standard pattern name */ dd_list *data; /* duration_data list */ - int pattern_duration; - int len; /* string length of pattern */ + unsigned int pattern_duration; int unlimit; }; struct duration_data { int duration; int intensity; - int frequency; - int overdriving; int wait; }; @@ -71,7 +68,7 @@ static int insert_conf_data(dd_list **conf_data, struct duration_data *update) { struct duration_data *data; - if (update->duration < 0 || update->intensity < 0 || update->frequency < 0 || update->overdriving < 0) + if (update->duration < 0 || update->intensity < 0) return 0; data = (struct duration_data *)calloc(1, sizeof(struct duration_data)); @@ -87,73 +84,63 @@ static int insert_conf_data(dd_list **conf_data, struct duration_data *update) return 0; } +static void get_pattern_property(char **iter, char property, int *value) +{ + char *check = strchr(*iter, property); + long int val; + + if (!check) + return; + + *check = '\0'; + val = strtol(*iter, NULL, 10); + if (val < 0) + val = 0; + else if (val > VIB_LOCK_TIMEOUT_MAX) + val = VIB_LOCK_TIMEOUT_MAX; + + *value = (int)val; + + *iter = check + 1; +} /* [A]xxxDxxxIxxxFxxxOxxxW format */ -static int insert_law_data_format(dd_list **conf_data, char *value) +static int insert_raw_data_format(dd_list **conf_data, const char *value) { - struct duration_data update; - int len = 0; - char *check; + struct duration_data update = {0, }; + char *iter; + char *end; int pattern_duration = 0; - if (!value) { - memset(&update, 0, sizeof(struct duration_data)); + if (!value) return insert_conf_data(conf_data, &update); - } - len = strlen(value); - do { + iter = strdup(value); + end = iter + strlen(iter); + while (iter < end) { memset(&update, 0, sizeof(struct duration_data)); - check = strchr(value, 'D'); - if (check) { - *check = '\0'; - update.duration = strtol(value, NULL, 10); - pattern_duration += update.duration; - if (pattern_duration > VIB_LOCK_TIMEOUT_MAX) { - _D("reached max pattern duration"); - pattern_duration = VIB_LOCK_TIMEOUT_MAX; - } - len = len - strlen(value) - 1; - value = check + 1; - check = strchr(value, 'I'); - if (check) { - *check = '\0'; - update.intensity = strtol(value, NULL, 10); - len = len - strlen(value) - 1; - value = check + 1; - } - check = strchr(value, 'F'); - if (check) { - *check = '\0'; - update.frequency = strtol(value, NULL, 10); - len = len - strlen(value) - 1; - value = check + 1; - } - check = strchr(value, 'O'); - if (check) { - *check = '\0'; - update.overdriving = strtol(value, NULL, 10); - len = len - strlen(value) - 1; - value = check + 1; - } - } - check = strchr(value, 'W'); - if (check) { - *check = '\0'; - update.wait = strtol(value, NULL, 10); - pattern_duration += update.wait; - if (pattern_duration > VIB_LOCK_TIMEOUT_MAX) { - _D("reached max pattern duration"); - pattern_duration = VIB_LOCK_TIMEOUT_MAX; - } - len = len - strlen(value) - 1; - value = check + 1; - } + get_pattern_property(&iter, 'D', &update.duration); + if (update.duration > VIB_LOCK_TIMEOUT_MAX) + update.duration = VIB_LOCK_TIMEOUT_MAX; + get_pattern_property(&iter, 'I', &update.intensity); + if (update.intensity > INTENSITY_BASE_RATE) + update.intensity = INTENSITY_BASE_RATE; + get_pattern_property(&iter, 'W', &update.wait); + if (update.wait > VIB_LOCK_TIMEOUT_MAX) + update.wait = VIB_LOCK_TIMEOUT_MAX; + if (update.duration == 0 && update.wait == 0) break; + + pattern_duration += (update.duration + update.wait); + if (pattern_duration > VIB_LOCK_TIMEOUT_MAX) { + _D("Max pattern duration"); + pattern_duration = VIB_LOCK_TIMEOUT_MAX; + } + if (insert_conf_data(conf_data, &update) < 0) return -EINVAL; - } while (len > 0); + } return pattern_duration; } @@ -161,17 +148,19 @@ static int insert_law_data_format(dd_list **conf_data, char *value) /* duration, intensity, frequency, overdriving waiting duration, intensity=0, frequency, overdriving - 85,10000,205,0, - 90,0,205,0, - 105,10000,205,0, + 85,10000, + 90,0, + 105,10000, 0,end */ -static int load_standard_format(char *pattern) +static int load_standard_format(const char *pattern) { struct vibration_config *conf; struct duration_data update = {0, }; bool packed = false; - int duration = 0, intensity = 0, ret = 0, count = 0, end = 2; + long int duration = 0, intensity = 0; + int ret = 0, count = 0, end = 2; + int index = 0; int fd; char path[PATH_MAX], elem, val[VALUE_MAX_LEN] = {0, }; @@ -193,13 +182,12 @@ static int load_standard_format(char *pattern) ret = -errno; goto error_out; } - conf->len = strlen(conf->pattern) + 1; /* make feedback pattern(xDxIxFxOxW) format from d,i,f,o, */ while (read(fd, &elem, 1) != 0) { if (end == 0) { if (insert_conf_data(&conf->data, &update) < 0) - goto error_out; + goto error_out; break; } if (elem == 'e' || elem == 'n' || elem == 'd') { @@ -212,22 +200,32 @@ static int load_standard_format(char *pattern) } if (elem == ',') { count++; - val[ret] = '\0'; - ret = 0; + val[index] = '\0'; + index = 0; switch (count) { case 1: /* D */ duration = strtol(val, NULL, 10); - conf->pattern_duration += duration; + if (duration < 0) + duration = 0; + else if (duration > VIB_LOCK_TIMEOUT_MAX) + duration = VIB_LOCK_TIMEOUT_MAX; + conf->pattern_duration += (int)duration; if (conf->pattern_duration > VIB_LOCK_TIMEOUT_MAX) { _D("Reached max pattern duration"); conf->pattern_duration = VIB_LOCK_TIMEOUT_MAX; } break; case 2: /* I or W */ + count = 0; intensity = strtol(val, NULL, 10); + if (intensity < 0) + intensity = 0; + else if (intensity > INTENSITY_BASE_RATE) + intensity = INTENSITY_BASE_RATE; if (!packed) { - update.duration = duration; - update.intensity = intensity; + update.duration = (int)duration; + update.intensity = (int)intensity; + packed = true; break; } /* Intensity should be 0 for wait(off) time */ @@ -241,28 +239,17 @@ static int load_standard_format(char *pattern) goto error_out; memset(&update, 0, sizeof(struct duration_data)); break; - case 3: /* F */ - if (intensity != 0) - update.frequency = strtol(val, NULL, 10); - break; - case 4: /* O */ - count = 0; - if (intensity != 0) { - packed = true; - update.overdriving = strtol(val, NULL, 10); - } - break; default: break; } } else { - if (ret < (VALUE_MAX_LEN - 2)) - val[ret++] = elem; + if (index < (VALUE_MAX_LEN - 2)) /* Temporal limit */ + val[index++] = elem; } } close(fd); DD_LIST_APPEND(vib_conf_list, conf); - return 0; + return ret; error_out: if (fd >= 0) @@ -281,6 +268,7 @@ static int vibration_load_config(struct parse_result *result, void *user_data) char *value; char *check; int len; + int duration; if (!result) return 0; @@ -306,13 +294,12 @@ static int vibration_load_config(struct parse_result *result, void *user_data) _E("fail to copy %s pattern data", result->name); goto error_out; } - conf->len = strlen(conf->pattern) + 1; value = result->value; len = strlen(value); if (len == 0) { - if (insert_law_data_format(&conf->data, NULL) < 0) + if (insert_raw_data_format(&conf->data, NULL) < 0) goto error_out; DD_LIST_APPEND(vib_conf_list, conf); return 0; @@ -346,11 +333,12 @@ static int vibration_load_config(struct parse_result *result, void *user_data) len = len - strlen(value) - 1; value = check + 1; } - conf->pattern_duration = insert_law_data_format(&conf->data, value); - if (conf->pattern_duration < 0) { + duration = insert_raw_data_format(&conf->data, value); + if (duration < 0) { conf->pattern_duration = 0; goto error_out; - } + } else + conf->pattern_duration = duration; DD_LIST_APPEND(vib_conf_list, conf); return 0; @@ -369,6 +357,7 @@ static void load_standard_vibration_patterns(void) { DIR *dir; struct dirent *dent; + int ret; dir = opendir(STANDARD_FILE_PATH); if (!dir) { @@ -378,7 +367,9 @@ static void load_standard_vibration_patterns(void) while ((dent = readdir(dir))) { if (dent->d_type == DT_DIR) continue; - load_standard_format(dent->d_name); + ret = load_standard_format(dent->d_name); + if (ret < 0) + _E("Failed to parse %s: %d", dent->d_name, ret); } closedir(dir); _D("Success to load %s", STANDARD_FILE_PATH); @@ -399,16 +390,14 @@ int standard_is_supported(const char *pattern) { dd_list *elem; struct vibration_config *conf; - size_t len; int ret; if (!pattern) return -EINVAL; - len = strlen(pattern) + 1; ret = 0; DD_LIST_FOREACH(vib_conf_list, elem, conf) { - if (!conf->pattern || conf->len != len) + if (!conf->pattern) continue; if (!strcmp(conf->pattern, pattern)) { ret = true; @@ -451,9 +440,8 @@ static gboolean haptic_duration_play(void *data) } DD_LIST_FOREACH_SAFE(head, n, next, node) { - _D("Handle %d play: %dms and Wait: %dms (with f:%d o:%d) %s type", + _D("Handle %d play: %dms and Wait: %dms %s type", cur_h_data.handle, node->duration, node->wait, - node->frequency, node->overdriving, cur_h_data.unlimit ? "Unlimit" : "Once"); if ((node->duration + node->wait) <= 0) { if (!cur_h_data.unlimit) { @@ -536,7 +524,7 @@ int standard_vibrate_effect(int device_handle, const char *requested_pattern, in break; if (!cur_h_data.unlimit && conf->pattern_duration > 0) { - ret = device_power_request_lock(POWER_LOCK_CPU, conf->pattern_duration); + ret = device_power_request_lock(POWER_LOCK_CPU, (int)conf->pattern_duration); if (ret != DEVICE_ERROR_NONE) _E("Failed to request power lock"); } -- 2.7.4