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 8ef452009249fdcce98d30c5b42089e111173758..ed160f8e5bbacdde979cdd2cc8689d786e66c8ab 100644 (file)
--- 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 413c974fb415223024a27a91347c94969433ca7e..0b930d579c3defc7ec19dc0b81de6bb10a07b3d9 100644 (file)
--- 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 66542469e321c630c87b2396c9d6ab0f9a84c3e2..1166fcd280b04ec05dda3ac7dc7033d41133f666 100644 (file)
--- 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 dbc61345a85869eda4c0d1eb7f0f0aa04fe4c9ce..6010d77313bb81a23f1442c15e6af808690745d3 100644 (file)
--- 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);
 }