From: Ben Pfaff Date: Sun, 27 Aug 2017 19:30:50 +0000 (-0700) Subject: sys-file-reader: Better handle duplicate names without long names. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?p=pspp;a=commitdiff_plain;h=7bf210c4fd179a22dd8c6a071f0b23f7ae4e14c2 sys-file-reader: Better handle duplicate names without long names. The reader did not properly handle the case where variable short names had duplicates and the system file did not include long names. In this case, the reader still tried to use the (duplicate) short names as the long names, which could in some cases cause a secondary name collision in the no-long-names case in parse_long_var_names(), and assert-failed. This commit first fixes the handling of duplicate short names: it sets the long name to the unique chosen short name, instead of the original duplicate short name. Then as an additional measure it refactors the code a little to always handle duplicates in rename_var_and_save_short_names(). I am not sure that that is necessary but it's a little bit of a code cleanup anyhow. CVE-2017-12960. See also https://bugzilla.redhat.com/show_bug.cgi?id=1482433. See also http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2017-12960. See also http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-12960. Found by team OWL337, using the collAFL fuzzer. --- diff --git a/NEWS b/NEWS index 8ef4520092..ed160f8e5b 100644 --- a/NEWS +++ b/NEWS @@ -6,7 +6,8 @@ Please send PSPP bug reports to bug-gnu-pspp@gnu.org. Changes since 1.0.0: - * Bug fixes, including fixes for CVE-2017-12958 and CVE-2017-12959. + * Bug fixes, including fixes for CVE-2017-12958, CVE-2017-12959, and + CVE-2017-12960. Changes from 0.11.0 to 1.0.0: diff --git a/src/data/dictionary.c b/src/data/dictionary.c index 413c974fb4..0b930d579c 100644 --- a/src/data/dictionary.c +++ b/src/data/dictionary.c @@ -756,17 +756,18 @@ rename_var (struct variable *v, const char *new_name) var_set_vardict (v, vardict); } -/* Changes the name of V in D to name NEW_NAME. Assert-fails if - a variable named NEW_NAME is already in D, except that - NEW_NAME may be the same as V's existing name. */ -void -dict_rename_var (struct dictionary *d, struct variable *v, - const char *new_name) +/* Tries to changes the name of V in D to name NEW_NAME. Returns true if + successful, false if a variable (other than V) with the given name already + exists in D. */ +bool +dict_try_rename_var (struct dictionary *d, struct variable *v, + const char *new_name) { - struct variable *old = var_clone (v); - assert (!utf8_strcasecmp (var_get_name (v), new_name) - || dict_lookup_var (d, new_name) == NULL); + struct variable *conflict = dict_lookup_var (d, new_name); + if (conflict && v != conflict) + return false; + struct variable *old = var_clone (v); unindex_var (d, var_get_vardict (v)); rename_var (v, new_name); reindex_var (d, var_get_vardict (v)); @@ -779,6 +780,19 @@ dict_rename_var (struct dictionary *d, struct variable *v, d->callbacks->var_changed (d, var_get_dict_index (v), VAR_TRAIT_NAME, old, d->cb_data); var_destroy (old); + + return true; +} + +/* Changes the name of V in D to name NEW_NAME. Assert-fails if + a variable named NEW_NAME is already in D, except that + NEW_NAME may be the same as V's existing name. */ +void +dict_rename_var (struct dictionary *d, struct variable *v, + const char *new_name) +{ + bool ok UNUSED = dict_try_rename_var (d, v, new_name); + assert (ok); } /* Renames COUNT variables specified in VARS to the names given diff --git a/src/data/dictionary.h b/src/data/dictionary.h index 66542469e3..1166fcd280 100644 --- a/src/data/dictionary.h +++ b/src/data/dictionary.h @@ -82,6 +82,8 @@ void dict_reorder_vars (struct dictionary *, struct variable *const *, size_t count); /* Variable names. */ +bool dict_try_rename_var (struct dictionary *, + struct variable *, const char *); void dict_rename_var (struct dictionary *, struct variable *, const char *); bool dict_rename_vars (struct dictionary *, struct variable **, char **new_names, diff --git a/src/data/sys-file-reader.c b/src/data/sys-file-reader.c index dbc61345a8..6010d77313 100644 --- a/src/data/sys-file-reader.c +++ b/src/data/sys-file-reader.c @@ -1457,11 +1457,13 @@ parse_variable_records (struct sfm_reader *r, struct dictionary *dict, "`%s' to `%s'."), name, new_name); var = rec->var = dict_create_var_assert (dict, new_name, rec->width); + var_set_short_name (var, 0, new_name); free (new_name); } - /* Set the short name the same as the long name. */ - var_set_short_name (var, 0, name); + /* Set the short name the same as the long name (even if we renamed + it). */ + var_set_short_name (var, 0, var_get_name (var)); /* Get variable label, if any. */ if (rec->label) @@ -1995,8 +1997,9 @@ parse_display_parameters (struct sfm_reader *r, } static void -rename_var_and_save_short_names (struct dictionary *dict, struct variable *var, - const char *new_name) +rename_var_and_save_short_names (struct sfm_reader *r, off_t pos, + struct dictionary *dict, + struct variable *var, const char *new_name) { size_t n_short_names; char **short_names; @@ -2014,7 +2017,8 @@ rename_var_and_save_short_names (struct dictionary *dict, struct variable *var, } /* Set long name. */ - dict_rename_var (dict, var, new_name); + if (!dict_try_rename_var (dict, var, new_name)) + sys_warn (r, pos, _("Duplicate long variable name `%s'."), new_name); /* Restore short names. */ for (i = 0; i < n_short_names; i++) @@ -2048,7 +2052,7 @@ parse_long_var_name_map (struct sfm_reader *r, char *new_name; new_name = utf8_to_lower (var_get_name (var)); - rename_var_and_save_short_names (dict, var, new_name); + rename_var_and_save_short_names (r, -1, dict, var, new_name); free (new_name); } @@ -2073,16 +2077,7 @@ parse_long_var_name_map (struct sfm_reader *r, continue; } - /* Identify any duplicates. */ - if (utf8_strcasecmp (var_get_short_name (var, 0), long_name) - && dict_lookup_var (dict, long_name) != NULL) - { - sys_warn (r, record->pos, - _("Duplicate long variable name `%s'."), long_name); - continue; - } - - rename_var_and_save_short_names (dict, var, long_name); + rename_var_and_save_short_names (r, record->pos, dict, var, long_name); } close_text_record (r, text); }