From 1cb715cef99a558b0fef35ea1a8b027f0a8dfa40 Mon Sep 17 00:00:00 2001 From: Otavio Pontes Date: Mon, 26 Nov 2018 23:48:44 +0000 Subject: [PATCH] helpers: Check for trailing characters in strtoi_err Besides checking for overflow, also checks if the number in the string doesn't have any trailing character. For example "123abc" will return an error and "123 abc" won't. Also: - Don't return EOVERFLOW anymore, because from the user of strtoi_err perspective it doesn't matter if the overflow is in the int size or in the long size. I would expect the same return code. Signed-off-by: Otavio Pontes --- src/filedesc.c | 2 +- src/globals.c | 2 +- src/helpers.c | 50 +++++++++++++++++++++++++++++++++++++++++--------- src/manifest.c | 6 +++--- src/search.c | 2 +- src/swupd.h | 3 ++- src/version.c | 6 +++--- 7 files changed, 52 insertions(+), 19 deletions(-) diff --git a/src/filedesc.c b/src/filedesc.c index 8bf6e164..5bc6416b 100644 --- a/src/filedesc.c +++ b/src/filedesc.c @@ -73,7 +73,7 @@ static void foreach_open_fd(void(pf)(int, void *), void *arg) continue; } - err = strtoi_err(entry->d_name, &ep, &n); + err = strtoi_err_endptr(entry->d_name, &ep, &n); if (err != 0) { fprintf(stderr, "Warning: invalid fd\n"); } diff --git a/src/globals.c b/src/globals.c index 2f5f0987..3b7cfc84 100644 --- a/src/globals.c +++ b/src/globals.c @@ -148,7 +148,7 @@ int get_version_from_path(const char *abs_path) ret = get_value_from_path(&ret_str, abs_path, true); if (ret == 0) { - int err = strtoi_err(ret_str, NULL, &val); + int err = strtoi_err(ret_str, &val); free_string(&ret_str); if (err != 0) { diff --git a/src/helpers.c b/src/helpers.c index 5ecc6a4d..387a5527 100644 --- a/src/helpers.c +++ b/src/helpers.c @@ -22,6 +22,7 @@ */ #define _GNU_SOURCE +#include #include #include #include @@ -602,14 +603,18 @@ out_fds: return ret; } -/* The strtol function is commonly used to convert a string to a number and +/* strtoi_err: Safely convert and string to integer avoiding overflows. + * + * The strtol function is commonly used to convert a string to a number and * the result is frequently stored in an int type, but type casting a long to - * an int can cause overflows. This function returns negative error codes based - * on the errno table. The -ERANGE code is returned when the string is out of - * range for strtol and the -EOVERFLOW code is returned when the long to int - * type conversion overflows. + * an int can cause overflows. + * + * This function returns negative error codes based on the errno table: + * -ERANGE is returned when the string is out of range for int value + * + * endptr is set with the value of the first invalid character in the string. */ -int strtoi_err(const char *str, char **endptr, int *value) +int strtoi_err_endptr(const char *str, char **endptr, int *value) { long num; int err; @@ -622,10 +627,10 @@ int strtoi_err(const char *str, char **endptr, int *value) * and return an overflow error code. */ if (num > INT_MAX) { num = INT_MAX; - err = -EOVERFLOW; + err = -ERANGE; } else if (num < INT_MIN) { num = INT_MIN; - err = -EOVERFLOW; + err = -ERANGE; } *value = (int)num; @@ -633,6 +638,33 @@ int strtoi_err(const char *str, char **endptr, int *value) return err; } +/* strtoi_err: Safely convert and string to integer avoiding overflows + * + * The strtol function is commonly used to convert a string to a number and + * the result is frequently stored in an int type, but type casting a long to + * an int can cause overflows. + * + * This function returns negative error codes based on the errno table: + * -ERANGE is returned when the string is out of range for int value + * -EINVAL is returned when the string isn't a valid number or has any invalid + * trailing character. +*/ +int strtoi_err(const char *str, int *value) +{ + char *endptr; + int err = strtoi_err_endptr(str, &endptr, value); + + if (err) { + return err; + } + + if (*endptr != '\0' && !isspace(*endptr)) { + return -EINVAL; + } + + return 0; +} + /* Return a duplicated copy of the string using strdup(). * Abort if there's no memory to allocate the new string. */ @@ -979,7 +1011,7 @@ bool on_new_format(void) return false; } - err = strtoi_err(ret_str, NULL, &res); + err = strtoi_err(ret_str, &res); free_string(&ret_str); if (err != 0) { diff --git a/src/manifest.c b/src/manifest.c index f6ee0f65..e0a125dc 100644 --- a/src/manifest.c +++ b/src/manifest.c @@ -131,7 +131,7 @@ static struct manifest *manifest_from_file(int version, char *component, bool he } c = &line[9]; - err = strtoi_err(c, NULL, &manifest_enc_version); + err = strtoi_err(c, &manifest_enc_version); if (manifest_enc_version <= 0 || err != 0) { fprintf(stderr, "Error: Loaded incompatible manifest version\n"); @@ -163,7 +163,7 @@ static struct manifest *manifest_from_file(int version, char *component, bool he } if (strncmp(line, "version:", 8) == 0) { - err = strtoi_err(c, NULL, &manifest_hdr_version); + err = strtoi_err(c, &manifest_hdr_version); if (manifest_hdr_version != version || err != 0) { fprintf(stderr, "Error: Loaded incompatible manifest header version\n"); goto err_close; @@ -330,7 +330,7 @@ static struct manifest *manifest_from_file(int version, char *component, bool he goto err; } - err = strtoi_err(c, NULL, &file->last_change); + err = strtoi_err(c, &file->last_change); if (file->last_change <= 0 || err != 0) { fprintf(stderr, "Error: Loaded incompatible manifest last change\n"); free(file); diff --git a/src/search.c b/src/search.c index 7973f423..99257a29 100644 --- a/src/search.c +++ b/src/search.c @@ -437,7 +437,7 @@ static bool parse_options(int argc, char **argv) break; case 't': - err = strtoi_err(optarg, NULL, &num_results); + err = strtoi_err(optarg, &num_results); if (err != 0) { fprintf(stderr, "Invalid --top argument\n\n"); goto err; diff --git a/src/swupd.h b/src/swupd.h index e2d383cd..8a6fe84e 100644 --- a/src/swupd.h +++ b/src/swupd.h @@ -365,7 +365,8 @@ extern int rm_bundle_file(const char *bundle); extern void print_manifest_files(struct manifest *m); extern void swupd_deinit(void); extern int swupd_init(void); -extern int strtoi_err(const char *str, char **endptr, int *value); +extern int strtoi_err(const char *str, int *value); +extern int strtoi_err_endptr(const char *str, char **endptr, int *value); extern void string_or_die(char **strp, const char *fmt, ...); char *strdup_or_die(const char *const str); extern void free_string(char **s); diff --git a/src/version.c b/src/version.c index df9c735f..45928e87 100644 --- a/src/version.c +++ b/src/version.c @@ -69,7 +69,7 @@ int get_latest_version(char *v_url) goto out; } else { tmp_version.data[tmp_version.len] = '\0'; - err = strtoi_err(tmp_version.data, NULL, &ret); + err = strtoi_err(tmp_version.data, &ret); if (err != 0) { ret = -1; @@ -123,7 +123,7 @@ int get_current_version(char *path_prefix) } *dest = 0; - err = strtoi_err(&line[11], NULL, &v); + err = strtoi_err(&line[11], &v); if (err != 0) { v = -1; } @@ -203,7 +203,7 @@ int read_mix_version_file(char *filename, char *path_prefix) *c = '\0'; } - err = strtoi_err(line, NULL, &v); + err = strtoi_err(line, &v); if (err != 0) { v = -1; }