From 7e3f95679677db07a5cc606f5edfb723ea56d04e Mon Sep 17 00:00:00 2001
From: Dana Robinson <derobins@hdfgroup.org>
Date: Mon, 26 Feb 2018 12:22:32 -0800
Subject: Fix for HDFFV-10354 (CVE-2017-17505).

---
 release_docs/RELEASE.txt | 23 +++++++++++++++++++++++
 src/H5Opline.c           | 44 +++++++++++++++++++++++++++++---------------
 2 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index c8e2bd5..eb9f0d9 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -132,6 +132,29 @@ Support for New Platforms, Languages, and Compilers
 Bug Fixes since HDF5-1.8.20
 ===========================
 
+    - If an HDF5 file contains a filter pipeline message with a 'number of
+      filters' field that exceeds the maximum number of allowed filters,
+      the error handling code will attempt to dereference a NULL pointer.
+
+      This issue was reported to The HDF Group as issue #CVE-2017-17505.
+
+      NOTE: The HDF5 C library cannot produce such a file. This condition
+            should only occur in a corrupt (or deliberately altered) file
+            or a file created by third-party software.
+
+      This problem arose because the error handling code assumed that
+      the 'number of filters' field implied that a dynamic array of that
+      size had already been created and that the cleanup code should
+      iterate over that array and clean up each element's resources. If
+      an error occurred before the array has been allocated, this will
+      not be true.
+
+      This has been changed so that the number of filters is set to
+      zero on errors. Additionally, the filter array traversal in the
+      error handling code now requires that the filter array not be NULL.
+
+      (DER - 2018/02/06, HDFFV-10354)
+
     Configuration
     -------------
     - CMake
diff --git a/src/H5Opline.c b/src/H5Opline.c
index 42fb72c..fb08431 100644
--- a/src/H5Opline.c
+++ b/src/H5Opline.c
@@ -132,8 +132,15 @@ H5O_pline_decode(H5F_t H5_ATTR_UNUSED *f, hid_t H5_ATTR_UNUSED dxpl_id, H5O_t H5
 
     /* Number of filters */
     pline->nused = *p++;
-    if(pline->nused > H5Z_MAX_NFILTERS)
-	HGOTO_ERROR(H5E_PLINE, H5E_CANTLOAD, NULL, "filter pipeline message has too many filters")
+    if(pline->nused > H5Z_MAX_NFILTERS) {
+
+        /* Reset the number of filters used to avoid array traversal in error
+         * handling code.
+         */
+        pline->nused = 0;
+
+        HGOTO_ERROR(H5E_PLINE, H5E_CANTLOAD, NULL, "filter pipeline message has too many filters")
+    }
 
     /* Reserved */
     if(pline->version == H5O_PLINE_VERSION_1)
@@ -495,23 +502,30 @@ H5O_pline_reset(void *mesg)
 
     FUNC_ENTER_NOAPI_NOINIT_NOERR
 
+    /* NOTE: This function can be called during error processing from
+     *       other API calls so DO NOT ASSUME THAT ANY VALUES ARE SANE.
+     */
+
     HDassert(pline);
 
-    /* Free information for each filter */
-    for(i = 0; i < pline->nused; i++) {
-        if(pline->filter[i].name && pline->filter[i].name != pline->filter[i]._name)
-            HDassert((HDstrlen(pline->filter[i].name) + 1) > H5Z_COMMON_NAME_LEN);
-        if(pline->filter[i].name != pline->filter[i]._name)
-            pline->filter[i].name = (char *)H5MM_xfree(pline->filter[i].name);
-        if(pline->filter[i].cd_values && pline->filter[i].cd_values != pline->filter[i]._cd_values)
-            HDassert(pline->filter[i].cd_nelmts > H5Z_COMMON_CD_VALUES);
-        if(pline->filter[i].cd_values != pline->filter[i]._cd_values)
-            pline->filter[i].cd_values = (unsigned *)H5MM_xfree(pline->filter[i].cd_values);
-    } /* end for */
+    /* Free the filter information and array */
+    if (pline->filter) {
+
+        /* Free information for each filter */
+        for(i = 0; i < pline->nused; i++) {
+            if(pline->filter[i].name && pline->filter[i].name != pline->filter[i]._name)
+                HDassert((HDstrlen(pline->filter[i].name) + 1) > H5Z_COMMON_NAME_LEN);
+            if(pline->filter[i].name != pline->filter[i]._name)
+                pline->filter[i].name = (char *)H5MM_xfree(pline->filter[i].name);
+            if(pline->filter[i].cd_values && pline->filter[i].cd_values != pline->filter[i]._cd_values)
+                HDassert(pline->filter[i].cd_nelmts > H5Z_COMMON_CD_VALUES);
+            if(pline->filter[i].cd_values != pline->filter[i]._cd_values)
+                pline->filter[i].cd_values = (unsigned *)H5MM_xfree(pline->filter[i].cd_values);
+        } /* end for */
 
-    /* Free filter array */
-    if(pline->filter)
+        /* Free filter array */
         pline->filter = (H5Z_filter_info_t *)H5MM_xfree(pline->filter);
+    }
 
     /* Reset # of filters */
     pline->nused = pline->nalloc = 0;
-- 
cgit v0.12