Merge pull request #2654 in HDFFV/hdf5 from bmr_HDFFV-10591 to develop

Fixed HDFFV-10591

* commit '7d58d115079d286a566207c529e59bbccc952b03':
  Added note for HDFFV-10591.
  Fix HDFFV-10591

4 files changed, 41 insertions, 12 deletions

diff --git a/hl/src/H5IM.c b/hl/src/H5IM.c
index 2a7ed9b..6f7414b 100644
--- a/hl/src/H5IM.c
+++ b/hl/src/H5IM.c
@@ -274,7 +274,8 @@ herr_t H5IMget_image_info( hid_t loc_id,
         return -1;
 
     /* Try to find the attribute "INTERLACE_MODE" on the >>image<< dataset */
-    has_attr = H5LT_find_attribute(did, "INTERLACE_MODE");
+    if ((has_attr = H5LT_find_attribute(did, "INTERLACE_MODE")) < 0)
+        goto out;
 
     /* It exists, get it */
     if(has_attr == 1)
diff --git a/hl/tools/gif2h5/hdf2gif.c b/hl/tools/gif2h5/hdf2gif.c
index e32facb..0e2a898 100644
--- a/hl/tools/gif2h5/hdf2gif.c
+++ b/hl/tools/gif2h5/hdf2gif.c
@@ -149,30 +149,41 @@ int main(int argc , char **argv)
             goto out;
         }
 
-        /* read image */
+        /* get image's information */
         if ( H5IMget_image_info( fid, image_name, &width, &height, &planes, interlace, &npals ) < 0 )
+        {
+            fprintf(stderr , "Unable to get information of the image. Aborting.\n");
             goto out;
+        }
 
-	if (width > IMAGE_WIDTH_MAX || height > IMAGE_HEIGHT_MAX){
-	    fprintf(stderr, "HDF5 image is too large. Limit is %d by %d.\n", IMAGE_WIDTH_MAX, IMAGE_HEIGHT_MAX);
-	    goto out;
-	}
+    	if (width > IMAGE_WIDTH_MAX || height > IMAGE_HEIGHT_MAX)
+        {
+	        fprintf(stderr, "HDF5 image is too large. Limit is %d by %d.\n", IMAGE_WIDTH_MAX, IMAGE_HEIGHT_MAX);
+	        goto out;
+	    }
 
-	/* tool can handle single plane images only. */
-	if (planes > 1){
-	    fprintf(stderr, "Cannot handle multiple planes image\n");
-	    goto out;
-	}
+	    /* tool can handle single plane images only. */
+	    if (planes > 1)
+        {
+	        fprintf(stderr, "Cannot handle multiple planes image\n");
+	        goto out;
+	    }
 
         Image = (GIFBYTE*) malloc( (size_t) width * (size_t) height );
 
         if ( H5IMread_image( fid, image_name, Image ) < 0 )
+        {
+            fprintf(stderr , "Unable to read the image. Aborting.\n");
             goto out;
+        }
 
         if (npals)
         {
             if ( H5IMget_palette_info( fid, image_name, 0, pal_dims ) < 0 )
+            {
+                fprintf(stderr , "Unable to get information of the palette. Aborting.\n");
                 goto out;
+            }
 
             pal = (GIFBYTE*) malloc( (size_t) pal_dims[0] * (size_t) pal_dims[1] );
 
@@ -246,7 +257,7 @@ int main(int argc , char **argv)
             if (j==i)
             {
                 /* wasn't found */
-	      pc2nc[i] = (GIFBYTE)nc;
+	            pc2nc[i] = (GIFBYTE)nc;
                 r1[nc] = Red[i];
                 g1[nc] = Green[i];
                 b1[nc] = Blue[i];
diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index 102402e..72cab28 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -631,6 +631,18 @@ Bug Fixes since HDF5-1.10.3 release
 
     Library
     -------
+    - Fixed the decoding of an attribute message to prevent a segfault by h52gif
+
+      The tool h52gif produced a segfault when the size of an attribute
+      message was corrupted and caused a buffer overflow.
+
+      The problem was fixed by verifying the attribute message's size
+      against the buffer size before accessing the buffer.  h52gif was
+      also fixed to display the failure instead of silently exiting
+      after the segfault was eliminated.
+
+      (BMR - 2020/6/19, HDFFV-10591)
+
     - Improved peformance when creating a large number of small datasets by
       retrieving default property values from the API context instead of doing
       skip list searches.
diff --git a/src/H5Oattr.c b/src/H5Oattr.c
index aeaebea..e38ef5c 100644
--- a/src/H5Oattr.c
+++ b/src/H5Oattr.c
@@ -238,6 +238,11 @@ H5O_attr_decode(H5F_t *f, H5O_t *open_oh, unsigned H5_ATTR_UNUSED mesg_flags,
 
     /* Go get the data */
     if(attr->shared->data_size) {
+        /* Ensure that data size doesn't exceed buffer size, in case of
+           it's being corrupted in the file */
+        if(attr->shared->data_size > p_size)
+            HGOTO_ERROR(H5E_RESOURCE, H5E_OVERFLOW, NULL, "data size exceeds buffer size")
+
         if(NULL == (attr->shared->data = H5FL_BLK_MALLOC(attr_buf, attr->shared->data_size)))
             HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")
         H5MM_memcpy(attr->shared->data, p, attr->shared->data_size);