From 6eba069093ef665b3fa5f179dd315ab89916c4e7 Mon Sep 17 00:00:00 2001 From: Jeff Morriss Date: Fri, 29 Oct 2010 22:09:31 +0000 Subject: [PATCH] Fix https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=4768 : Return an error if the user specifies a value in a range in excess of the range-specified maximum. Except when reading in preferences files which might have ranges that exceed the maximum (because we didn't use to check): in that case silently lower the out-of-range values. svn path=/trunk/; revision=34698 --- epan/prefs-int.h | 4 +-- epan/prefs.c | 25 +++++++++++------ epan/prefs.h | 2 +- epan/range.c | 71 ++++++++++++++++++++++++++++++++++-------------- epan/range.h | 15 ++++++---- 5 files changed, 79 insertions(+), 38 deletions(-) diff --git a/epan/prefs-int.h b/epan/prefs-int.h index ede8afccb7..8208bfa3d2 100644 --- a/epan/prefs-int.h +++ b/epan/prefs-int.h @@ -115,11 +115,11 @@ struct preference { /** * Given a string of the form ":", as might appear * as an argument to a "-o" option, parse it and set the preference in - * question. + * question. * @return an indication of whether it succeeded or failed * in some fashion. */ -typedef prefs_set_pref_e (*pref_set_pair_cb) (gchar *key, gchar *value, void *private_data); +typedef prefs_set_pref_e (*pref_set_pair_cb) (gchar *key, gchar *value, void *private_data, gboolean return_range_errors); /** read the preferences file (or similiar) and call the callback * function to set each key/value pair found diff --git a/epan/prefs.c b/epan/prefs.c index 10b7bbb722..f98ed4194b 100644 --- a/epan/prefs.c +++ b/epan/prefs.c @@ -59,7 +59,7 @@ static module_t *find_subtree(module_t *parent, const char *tilte); static module_t *prefs_register_module_or_subtree(module_t *parent, const char *name, const char *title, const char *description, gboolean is_subtree, void (*apply_cb)(void)); -static prefs_set_pref_e set_pref(gchar*, gchar*, void *); +static prefs_set_pref_e set_pref(gchar*, gchar*, void *, gboolean); static gchar *put_string_list(GList *); static void free_col_info(e_prefs *); @@ -1477,7 +1477,11 @@ read_prefs_file(const char *pf_path, FILE *pf, if (isalnum(got_c)) { if (cur_var->len > 0) { if (got_val) { - switch (pref_set_pair_fct(cur_var->str, cur_val->str, private_data)) { + /* Convert the string to a range. Since we're reading the + * preferences file, silently lower values in excess of the + * range's maximum. + */ + switch (pref_set_pair_fct(cur_var->str, cur_val->str, private_data, FALSE)) { case PREFS_SET_OK: break; @@ -1537,7 +1541,11 @@ read_prefs_file(const char *pf_path, FILE *pf, } if (cur_var->len > 0) { if (got_val) { - switch (pref_set_pair_fct(cur_var->str, cur_val->str, private_data)) { + /* Convert the string to a range. Since we're reading the + * preferences file, silently lower values in excess of the + * range's maximum. + */ + switch (pref_set_pair_fct(cur_var->str, cur_val->str, private_data, FALSE)) { case PREFS_SET_OK: break; @@ -1663,7 +1671,7 @@ prefs_set_pref(char *prefarg) return PREFS_SET_SYNTAX_ERR; } if (strcmp(prefarg, "uat")) { - ret = set_pref(prefarg, p, NULL); + ret = set_pref(prefarg, p, NULL, TRUE); } else { ret = prefs_set_uat_pref(p) ? PREFS_SET_OK : PREFS_SET_SYNTAX_ERR; } @@ -1944,7 +1952,8 @@ try_convert_to_custom_column(gpointer *el_data) } static prefs_set_pref_e -set_pref(gchar *pref_name, gchar *value, void *private_data _U_) +set_pref(gchar *pref_name, gchar *value, void *private_data _U_, + gboolean return_range_errors) { GList *col_l, *col_l_elt; gint llen; @@ -2695,10 +2704,8 @@ set_pref(gchar *pref_name, gchar *value, void *private_data _U_) { range_t *newrange; - if (range_convert_str(&newrange, value, pref->info.max_value) != - CVT_NO_ERROR) { - /* XXX - distinguish between CVT_SYNTAX_ERROR and - CVT_NUMBER_TOO_BIG */ + if (range_convert_str_work(&newrange, value, pref->info.max_value, + return_range_errors) != CVT_NO_ERROR) { return PREFS_SET_SYNTAX_ERR; /* number was bad */ } diff --git a/epan/prefs.h b/epan/prefs.h index c15b966afa..3cfa157042 100644 --- a/epan/prefs.h +++ b/epan/prefs.h @@ -301,7 +301,7 @@ extern const char *prefs_get_title_by_name(const char *name); */ extern module_t *prefs_find_module(const char *name); -/** Given a module name, and a preference name return a pointer to the given +/** Given a module name, and a preference name return a pointer to the given * module's given preference or NULL if it's not found. * * @param module The preference module name. Usually the same as the protocol diff --git a/epan/range.c b/epan/range.c index 2f38b24716..ffa1ab11f8 100644 --- a/epan/range.c +++ b/epan/range.c @@ -67,12 +67,12 @@ range_t *range_empty(void) * of ranges specified, and fills the array range->ranges containing * low and high values with the number of ranges being range->nranges. * After having called this function, the function value_is_in_range() - * determines whether a given number is within the range or not. + * determines whether a given number is within the range or not. * - * In case of a single number, we make a range where low is equal to high. + * In case of a single number, we make a range where low is equal to high. * We take care on wrongly entered ranges; opposite order will be taken * care of. - * + * * The following syntax is accepted : * * 1-20,30-40 Range from 1 to 20, and packets 30 to 40 @@ -82,9 +82,22 @@ range_t *range_empty(void) * - All values */ -convert_ret_t range_convert_str(range_t **rangep, const gchar *es, - guint32 max_value) +convert_ret_t +range_convert_str(range_t **rangep, const gchar *es, guint32 max_value) { + return range_convert_str_work(rangep, es, max_value, TRUE); +} + +/* This version of range_convert_str() allows the caller to specify whether + * values in excess of the range's specified maximum should cause an error or + * be silently lowered. + * XXX - both the function and the variable could probably use better names. + */ +convert_ret_t +range_convert_str_work(range_t **rangep, const gchar *es, guint32 max_value, + gboolean err_on_max) +{ + range_t *range; guint nranges; const gchar *p; @@ -142,11 +155,18 @@ convert_ret_t range_convert_str(range_t **rangep, const gchar *es, g_free(range); return CVT_SYNTAX_ERROR; } - if (errno == ERANGE || val > G_MAXUINT32) { - /* That was valid, but it's too big. */ - g_free(range); - return CVT_NUMBER_TOO_BIG; - } + if (errno == ERANGE || val > max_value) { + /* That was valid, but it's too big. Return an error if requested + * (e.g., except when reading from the preferences file). + */ + if (err_on_max) { + g_free(range); + return CVT_NUMBER_TOO_BIG; + } else { + /* Silently use the range's maximum value */ + val = max_value; + } + } p = endp; range->ranges[range->nranges].low = val; @@ -181,11 +201,18 @@ convert_ret_t range_convert_str(range_t **rangep, const gchar *es, g_free(range); return CVT_SYNTAX_ERROR; } - if (errno == ERANGE || val > G_MAXUINT32) { - /* That was valid, but it's too big. */ - g_free(range); - return CVT_NUMBER_TOO_BIG; - } + if (errno == ERANGE || val > max_value) { + /* That was valid, but it's too big. Return an error if requested + * (e.g., except when reading from the preferences file). + */ + if (err_on_max) { + g_free(range); + return CVT_NUMBER_TOO_BIG; + } else { + /* Silently use the range's maximum value */ + val = max_value; + } + } p = endp; range->ranges[range->nranges].high = val; @@ -203,7 +230,7 @@ convert_ret_t range_convert_str(range_t **rangep, const gchar *es, */ range->ranges[range->nranges].high = range->ranges[range->nranges].low; } else { - /* Invalid character. */ + /* Invalid character. */ g_free(range); return CVT_SYNTAX_ERROR; } @@ -241,7 +268,8 @@ convert_ret_t range_convert_str(range_t **rangep, const gchar *es, /* This function returns TRUE if a given value is within one of the ranges * stored in the ranges array. */ -gboolean value_is_in_range(range_t *range, guint32 val) +gboolean +value_is_in_range(range_t *range, guint32 val) { guint i; @@ -256,7 +284,8 @@ gboolean value_is_in_range(range_t *range, guint32 val) /* This function returns TRUE if the two given range_t's are equal. */ -gboolean ranges_are_equal(range_t *a, range_t *b) +gboolean +ranges_are_equal(range_t *a, range_t *b) { guint i; @@ -312,7 +341,8 @@ range_convert_range(range_t *range) } /* Create a copy of a range. */ -range_t *range_copy(range_t *src) +range_t * +range_copy(range_t *src) { range_t *dst; size_t range_size; @@ -325,7 +355,8 @@ range_t *range_copy(range_t *src) #if 0 /* This is a debug function to check the range functionality */ -static void value_is_in_range_check(range_t *range, guint32 val) +static void +value_is_in_range_check(range_t *range, guint32 val) { /* Print the result for a given value */ diff --git a/epan/range.h b/epan/range.h index 26ecc5e42a..16297b03b3 100644 --- a/epan/range.h +++ b/epan/range.h @@ -58,7 +58,7 @@ typedef enum { CVT_NO_ERROR, CVT_SYNTAX_ERROR, CVT_NUMBER_TOO_BIG -} convert_ret_t; +} convert_ret_t; extern range_t *range_empty(void); @@ -69,10 +69,10 @@ extern range_t *range_empty(void); * low and high values with the number of ranges being range->nranges. * After having called this function, the function value_is_in_range() * determines whether a given number is within the range or not.
- * In case of a single number, we make a range where low is equal to high. + * In case of a single number, we make a range where low is equal to high. * We take care on wrongly entered ranges; opposite order will be taken * care of. - * + * * The following syntax is accepted : * * 1-20,30-40 Range from 1 to 20, and packets 30 to 40 @@ -83,11 +83,14 @@ extern range_t *range_empty(void); * @param range the range * @param es points to the string to be converted. * @param max_value' specifies the maximum value in a range. - * @return + * @return */ extern convert_ret_t range_convert_str(range_t **range, const gchar *es, guint32 max_value); +extern convert_ret_t range_convert_str_work(range_t **range, const gchar *es, + guint32 max_value, gboolean err_on_max); + /** This function returns TRUE if a given value is within one of the ranges * stored in the ranges array. * @param range the range @@ -110,8 +113,8 @@ extern gboolean ranges_are_equal(range_t *a, range_t *b); */ extern void range_foreach(range_t *range, void (*callback)(guint32 val)); -/** - * This function converts a range_t to a (ep_alloc()-allocated) string. +/** + * This function converts a range_t to a (ep_alloc()-allocated) string. */ extern char *range_convert_range(range_t *range);