sys-file-reader: Avoid assert-fail for duplicate attribute names.

CVE-2017-12961.
See also https://bugzilla.redhat.com/show_bug.cgi?id=1482436.
See also http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2017-12961.
See also http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-12961.
Found by team OWL337, using the collAFL fuzzer.

diff --git a/NEWS b/NEWS
index ed160f8e5bbacdde979cdd2cc8689d786e66c8ab..7914b0dc92867d7852e9ef5d65fbcbd21751cc4c 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -6,8 +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, CVE-2017-12959, and
-   CVE-2017-12960.
+ * Bug fixes, including fixes for CVE-2017-12958, CVE-2017-12959,
+   CVE-2017-12960, and CVE-2017-12961.
 
 Changes from 0.11.0 to 1.0.0:
 

diff --git a/src/data/attributes.c b/src/data/attributes.c
index f516dc61a172f72afb792f27026a109447b5c903..d7b45ff59571dd65567349d937f35ee4e038fa1d 100644 (file)
--- a/src/data/attributes.c
+++ b/src/data/attributes.c
@@ -22,6 +22,7 @@
 #include <string.h>
 
 #include "libpspp/array.h"
+#include "libpspp/compiler.h"
 #include "libpspp/hash-functions.h"
 #include "libpspp/i18n.h"
 
@@ -230,15 +231,28 @@ attrset_lookup (const struct attrset *set, const char *name)
   return CONST_CAST (struct attribute *, attr);
 }
 
+/* Adds ATTR to SET.  Succeeds and returns true if SET does not already contain
+   an attribute with the same name (matched case insensitively); otherwise
+   fails and returns false.  On success only, ownership of ATTR is transferred
+   to SET. */
+bool
+attrset_try_add (struct attrset *set, struct attribute *attr)
+{
+  const char *name = attribute_get_name (attr);
+  if (attrset_lookup (set, name))
+    return false;
+  hmap_insert (&set->map, &attr->node, utf8_hash_case_string (name, 0));
+  return true;
+}
+
 /* Adds ATTR to SET, which must not already contain an attribute
    with the same name (matched case insensitively).  Ownership of
    ATTR is transferred to SET. */
 void
 attrset_add (struct attrset *set, struct attribute *attr)
 {
-  const char *name = attribute_get_name (attr);
-  assert (attrset_lookup (set, name) == NULL);
-  hmap_insert (&set->map, &attr->node, utf8_hash_case_string (name, 0));
+  bool ok UNUSED = attrset_try_add (set, attr);
+  assert (ok);
 }
 
 /* Deletes any attribute from SET that matches NAME

diff --git a/src/data/attributes.h b/src/data/attributes.h
index b0712fcc8f4a8ab6f5f5d2ec1a9662b80fe4ea5c..56d142e4c23ceb375e7c11e220fa4f34cde62346 100644 (file)
--- a/src/data/attributes.h
+++ b/src/data/attributes.h
@@ -53,6 +53,7 @@ void attrset_destroy (struct attrset *);
 size_t attrset_count (const struct attrset *);
 
 struct attribute *attrset_lookup (const struct attrset *, const char *);
+bool attrset_try_add (struct attrset *, struct attribute *);
 void attrset_add (struct attrset *, struct attribute *);
 void attrset_delete (struct attrset *, const char *);
 void attrset_clear (struct attrset *);

diff --git a/src/data/sys-file-reader.c b/src/data/sys-file-reader.c
index 6010d77313bb81a23f1442c15e6af808690745d3..8abfe10b689fda6ce24df8c7d916ee3618a37aa7 100644 (file)
--- a/src/data/sys-file-reader.c
+++ b/src/data/sys-file-reader.c
@@ -2335,7 +2335,14 @@ parse_attributes (struct sfm_reader *r, struct text_record *text,
             break;
         }
       if (attrs != NULL)
-        attrset_add (attrs, attr);
+        {
+          if (!attrset_try_add (attrs, attr))
+            {
+              text_warn (r, text, _("Duplicate attribute %s."),
+                         attribute_get_name (attr));
+              attribute_destroy (attr);
+            }
+        }
       else
         attribute_destroy (attr);
     }

diff --git a/tests/data/sys-file-reader.at b/tests/data/sys-file-reader.at
index 8ca6d74494e846d1268b5afa5f7429c072590ac3..d071f37c09f97529d20934181e9635ee2f271010 100644 (file)
--- a/tests/data/sys-file-reader.at
+++ b/tests/data/sys-file-reader.at
@@ -3465,6 +3465,48 @@ warning: `sys-file.sav' near offset 0x106: Attribute value fred[[1]] is not quot
 done
 AT_CLEANUP
 
+AT_SETUP([duplicate attribute name])
+AT_KEYWORDS([sack synthetic system file negative])
+AT_DATA([sys-file.sack], [dnl
+dnl File header.
+"$FL2"; s60 "$(#) SPSS DATA FILE PSPP synthetic test file";
+2; 1; 1; 0; -1; 100.0; "01 Jan 11"; "20:53:52"; s64 ""; i8 0 *3;
+
+dnl Variables.
+2; 0; 0; 0; 0x050800 *2; s8 "FIRSTVAR";
+
+dnl Data file attributes record.
+7; 17; 1; COUNT (
+"Attr1('value'"; i8 10; ")";
+"Attr1('value'"; i8 10; ")";
+);
+
+dnl Variable attributes record.
+7; 18; 1; COUNT (
+"FIRSTVAR:";
+  "fred('23'"; i8 10; ")";
+  "fred('23'"; i8 10; ")";
+);
+
+dnl Character encoding record.
+7; 20; 1; 12; "windows-1252";
+
+dnl Dictionary termination record.
+999; 0;
+])
+for variant in be le; do
+  AT_CHECK([sack --$variant sys-file.sack > sys-file.sav])
+  AT_DATA([sys-file.sps], [dnl
+GET FILE='sys-file.sav'.
+])
+  AT_CHECK([pspp -O format=csv sys-file.sps], [0], [dnl
+warning: `sys-file.sav' near offset 0xf6: Duplicate attribute Attr1.
+
+warning: `sys-file.sav' near offset 0x125: Duplicate attribute fred.
+])
+done
+AT_CLEANUP
+
 AT_SETUP([bad variable name in long string value label])
 AT_KEYWORDS([sack synthetic system file negative])
 AT_DATA([sys-file.sack], [dnl