From 62b5101a28fc2c4a9b8b26a998fb6c4ec12d84c7 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Sun, 27 Aug 2017 12:31:05 -0700 Subject: [PATCH] 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. --- NEWS | 4 ++-- src/data/attributes.c | 20 ++++++++++++++--- src/data/attributes.h | 1 + src/data/sys-file-reader.c | 9 +++++++- tests/data/sys-file-reader.at | 42 +++++++++++++++++++++++++++++++++++ 5 files changed, 70 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index ed160f8e5b..7914b0dc92 100644 --- 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 f516dc61a1..d7b45ff595 100644 --- a/src/data/attributes.c +++ b/src/data/attributes.c @@ -22,6 +22,7 @@ #include #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 b0712fcc8f..56d142e4c2 100644 --- 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 6010d77313..8abfe10b68 100644 --- 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 8ca6d74494..d071f37c09 100644 --- 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 -- 2.30.2