From ca6f9e3716bd1a35624167986b836d2f7674bcd1 Mon Sep 17 00:00:00 2001
From: Dana Robinson <43805+derobins@users.noreply.github.com>
Date: Wed, 24 Nov 2021 14:20:13 -0800
Subject: Fixes an assert in H5Pget_filter_by_id1/2 w/ out-of-range IDs (#1222)

* Fixes an assert in H5Pget_filter_by_id1/2 w/ out-of-range IDs

Filter IDs < 0 or > H5Z_FILTER_MAX could trip an assert in the
library due to missing ID range checks in H5Pget_filter_by_id1/2.
The library now returns a normal error code when filter IDs are
out of range. Fixes HDFFV-11286.

* Committing clang-format changes

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
---
 release_docs/RELEASE.txt |  8 ++++
 src/H5Pocpl.c            |  4 ++
 test/filter_plugin.c     | 99 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 111 insertions(+)

diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index fbdbf3f..e035d03 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -1039,6 +1039,14 @@ Bug Fixes since HDF5-1.12.0 release
 ===================================
     Library
     -------
+    - Fixed an H5Pget_filter_by_id1/2() assert w/ out of range filter IDs
+
+      Both H5Pget_filter_by_id1 and 2 did not range check the filter ID, which
+      could trip as assert in debug versions of the library. The library now
+      returns a normal HDF5 error when the filter ID is out of range.
+
+      (DER - 2021/11/23, HDFFV-11286)
+
     - Fixed an issue with collective metadata reads being permanently disabled
       after a dataset chunk lookup operation. This would usually cause a
       mismatched MPI_Bcast and MPI_ERR_TRUNCATE issue in the library for
diff --git a/src/H5Pocpl.c b/src/H5Pocpl.c
index edb0cca..e442030 100644
--- a/src/H5Pocpl.c
+++ b/src/H5Pocpl.c
@@ -940,6 +940,8 @@ H5Pget_filter_by_id2(hid_t plist_id, H5Z_filter_t id, unsigned int *flags /*out*
     H5TRACE8("e", "iZfx*zxzxx", plist_id, id, flags, cd_nelmts, cd_values, namelen, name, filter_config);
 
     /* Check args */
+    if (id < 0 || id > H5Z_FILTER_MAX)
+        HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "filter ID value out of range")
     if (cd_nelmts || cd_values) {
         /*
          * It's likely that users forget to initialize this on input, so
@@ -1838,6 +1840,8 @@ H5Pget_filter_by_id1(hid_t plist_id, H5Z_filter_t id, unsigned int *flags /*out*
     H5TRACE7("e", "iZfx*zxzx", plist_id, id, flags, cd_nelmts, cd_values, namelen, name);
 
     /* Check args */
+    if (id < 0 || id > H5Z_FILTER_MAX)
+        HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "filter ID value out of range")
     if (cd_nelmts || cd_values) {
         /*
          * It's likely that users forget to initialize this on input, so
diff --git a/test/filter_plugin.c b/test/filter_plugin.c
index 276141a..c373b3b 100644
--- a/test/filter_plugin.c
+++ b/test/filter_plugin.c
@@ -1305,6 +1305,102 @@ error:
 } /* end test_path_api_calls() */
 
 /*-------------------------------------------------------------------------
+ * Function:  test_filter_numbers
+ *
+ * Purpose:   Tests the filter numbers are handled correctly
+ *
+ * Return:    SUCCEED/FAIL
+ *
+ *-------------------------------------------------------------------------
+ */
+static herr_t
+test_filter_numbers(void)
+{
+    hid_t        dcpl_id = H5I_INVALID_HID;
+    H5Z_filter_t id;
+    herr_t       status = SUCCEED;
+    size_t       nelmts = 0;
+    unsigned int flags;
+    unsigned int filter_config;
+
+    HDputs("Testing filter number handling");
+
+    /* Check that out-of-range filter numbers are handled correctly */
+    TESTING("    Filter # out of range");
+
+    /* Create property list */
+    if ((dcpl_id = H5Pcreate(H5P_DATASET_CREATE)) < 0)
+        TEST_ERROR;
+
+    nelmts = 0;
+
+    /* Test id > H5Z_FILTER_MAX and < 0, current version */
+
+    H5E_BEGIN_TRY
+    {
+        id     = H5Z_FILTER_MAX + 1;
+        status = H5Pget_filter_by_id2(dcpl_id, id, &flags, &nelmts, NULL, 0, NULL, &filter_config);
+    }
+    H5E_END_TRY;
+
+    /* Should fail */
+    if (status != FAIL)
+        TEST_ERROR;
+
+    H5E_BEGIN_TRY
+    {
+        id     = -1;
+        status = H5Pget_filter_by_id2(dcpl_id, id, &flags, &nelmts, NULL, 0, NULL, &filter_config);
+    }
+    H5E_END_TRY;
+
+    /* Should fail */
+    if (status != FAIL)
+        TEST_ERROR;
+
+        /* Test id > H5Z_FILTER_MAX and < 0, deprecated version */
+
+#ifndef H5_NO_DEPRECATED_SYMBOLS
+    H5E_BEGIN_TRY
+    {
+        id     = H5Z_FILTER_MAX + 1;
+        status = H5Pget_filter_by_id1(dcpl_id, id, &flags, &nelmts, NULL, 0, NULL);
+    }
+    H5E_END_TRY;
+
+    /* Should fail */
+    if (status != FAIL)
+        TEST_ERROR;
+
+    H5E_BEGIN_TRY
+    {
+        id     = -1;
+        status = H5Pget_filter_by_id1(dcpl_id, id, &flags, &nelmts, NULL, 0, NULL);
+    }
+    H5E_END_TRY;
+
+    /* Should fail */
+    if (status != FAIL)
+        TEST_ERROR;
+#endif
+
+    if (H5Pclose(dcpl_id) < 0)
+        TEST_ERROR;
+
+    PASSED();
+
+    return SUCCEED;
+
+error:
+    H5E_BEGIN_TRY
+    {
+        H5Pclose(dcpl_id);
+    }
+    H5E_END_TRY;
+    return FAIL;
+} /* end test_filter_numbers() */
+
+/*-------------------------------------------------------------------------
  * Function:  disable_chunk_cache
  *
  * Purpose:   Turns the chunk cache off
@@ -1523,6 +1619,9 @@ main(void)
     /* Test the APIs for access to the filter plugin path table */
     nerrors += (test_path_api_calls() < 0 ? 1 : 0);
 
+    /* Test filter numbers */
+    nerrors += (test_filter_numbers() < 0 ? 1 : 0);
+
     if (nerrors)
         TEST_ERROR;
 
-- 
cgit v0.12