Fix for HDFFV-10216 segfault in H5G_node_cmp3 with corrupt h5 file

Fix H5HL_offset_into() to return error when offset exceeds heap data block size.
Also fix other places that call this routine to detect error return.
This commit is contained in:
Vailin Choi 2017-08-22 01:36:20 -05:00
parent a6d5bf1a86
commit b07eb6efd0
12 changed files with 606 additions and 412 deletions

View File

@ -909,6 +909,7 @@
./test/atomic_reader.c
./test/atomic_writer.c
./test/bad_compound.h5
./test/bad_offset.h5
./test/be_data.h5
./test/be_extlink1.h5
./test/be_extlink2.h5
@ -972,6 +973,7 @@
./test/fsm_aggr_persist.h5
./test/genall5.c
./test/genall5.h
./test/gen_bad_offset.c
./test/gen_bad_ohdr.c
./test/gen_bad_compound.c
./test/gen_bogus.c

View File

@ -541,8 +541,8 @@ herr_t
H5G__ent_debug(const H5G_entry_t *ent, FILE *stream, int indent, int fwidth,
const H5HL_t *heap)
{
const char *lval = NULL;
int nested_indent, nested_fwidth;
const char *lval = NULL;
int nested_indent, nested_fwidth;
FUNC_ENTER_PACKAGE_NOERR
@ -551,14 +551,14 @@ H5G__ent_debug(const H5G_entry_t *ent, FILE *stream, int indent, int fwidth,
nested_fwidth = MAX(0, fwidth - 3);
HDfprintf(stream, "%*s%-*s %lu\n", indent, "", fwidth,
"Name offset into private heap:",
(unsigned long) (ent->name_off));
"Name offset into private heap:",
(unsigned long) (ent->name_off));
HDfprintf(stream, "%*s%-*s %a\n", indent, "", fwidth,
"Object header address:", ent->header);
"Object header address:", ent->header);
HDfprintf(stream, "%*s%-*s ", indent, "", fwidth,
"Cache info type:");
"Cache info type:");
switch(ent->type) {
case H5G_NOTHING_CACHED:
HDfprintf(stream, "Nothing Cached\n");
@ -581,13 +581,13 @@ H5G__ent_debug(const H5G_entry_t *ent, FILE *stream, int indent, int fwidth,
HDfprintf(stream, "%*s%-*s\n", indent, "", fwidth,
"Cached information:");
HDfprintf(stream, "%*s%-*s %lu\n", nested_indent, "", nested_fwidth,
"Link value offset:",
(unsigned long)(ent->cache.slink.lval_offset));
"Link value offset:",
(unsigned long)(ent->cache.slink.lval_offset));
if(heap) {
lval = (const char *)H5HL_offset_into(heap, ent->cache.slink.lval_offset);
HDfprintf(stream, "%*s%-*s %s\n", nested_indent, "", nested_fwidth,
"Link value:",
lval);
"Link value:",
(lval == NULL) ? "" : lval);
} /* end if */
else
HDfprintf(stream, "%*s%-*s\n", nested_indent, "", nested_fwidth, "Warning: Invalid heap address given, name not displayed!");

View File

@ -224,7 +224,9 @@ herr_t
H5G__ent_to_link(H5O_link_t *lnk, const H5HL_t *heap,
const H5G_entry_t *ent, const char *name)
{
FUNC_ENTER_PACKAGE_NOERR
herr_t ret_value = SUCCEED; /* Return value */
FUNC_ENTER_PACKAGE
/* check arguments */
HDassert(lnk);
@ -243,7 +245,8 @@ H5G__ent_to_link(H5O_link_t *lnk, const H5HL_t *heap,
if(ent->type == H5G_CACHED_SLINK) {
const char *s; /* Pointer to link value */
s = (const char *)H5HL_offset_into(heap, ent->cache.slink.lval_offset);
if((s = (const char *)H5HL_offset_into(heap, ent->cache.slink.lval_offset)) == NULL)
HGOTO_ERROR(H5E_LINK, H5E_CANTGET, FAIL, "unable to get link name")
HDassert(s);
/* Copy the link value */
@ -260,7 +263,10 @@ H5G__ent_to_link(H5O_link_t *lnk, const H5HL_t *heap,
lnk->type = H5L_TYPE_HARD;
} /* end else */
FUNC_LEAVE_NOAPI(SUCCEED)
done:
if(ret_value < 0 && lnk->name)
H5MM_xfree(lnk->name);
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5G__ent_to_link() */

File diff suppressed because it is too large Load Diff

View File

@ -707,11 +707,12 @@ done:
static herr_t
H5G_stab_get_name_by_idx_cb(const H5G_entry_t *ent, void *_udata)
{
H5G_bt_it_gnbi_t *udata = (H5G_bt_it_gnbi_t *)_udata;
H5G_bt_it_gnbi_t *udata = (H5G_bt_it_gnbi_t *)_udata;
size_t name_off; /* Offset of name in heap */
const char *name; /* Pointer to name string in heap */
herr_t ret_value = SUCCEED; /* Return value */
FUNC_ENTER_NOAPI_NOINIT_NOERR
FUNC_ENTER_NOAPI_NOINIT
/* Sanity check */
HDassert(ent);
@ -719,12 +720,16 @@ H5G_stab_get_name_by_idx_cb(const H5G_entry_t *ent, void *_udata)
/* Get name offset in heap */
name_off = ent->name_off;
name = (const char *)H5HL_offset_into(udata->heap, name_off);
if((name = (const char *)H5HL_offset_into(udata->heap, name_off)) == NULL)
HGOTO_ERROR(H5E_SYM, H5E_CANTGET, FAIL, "unable to get symbol table link name")
HDassert(name);
udata->name = H5MM_strdup(name);
HDassert(udata->name);
FUNC_LEAVE_NOAPI(SUCCEED)
done:
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5G_stab_get_name_by_idx_cb */
@ -941,7 +946,8 @@ H5G_stab_lookup_by_idx_cb(const H5G_entry_t *ent, void *_udata)
HDassert(udata && udata->heap);
/* Get a pointer to the link name */
name = (const char *)H5HL_offset_into(udata->heap, ent->name_off);
if((name = (const char *)H5HL_offset_into(udata->heap, ent->name_off)) == NULL)
HGOTO_ERROR(H5E_SYM, H5E_CANTGET, FAIL, "unable to get symbol table link name")
HDassert(name);
/* Convert the entry to a link */

View File

@ -400,18 +400,20 @@ END_FUNC(PRIV) /* end H5HL_protect() */
*
*-------------------------------------------------------------------------
*/
BEGIN_FUNC(PRIV, NOERR,
void *, NULL, -,
BEGIN_FUNC(PRIV, ERR,
void *, NULL, NULL,
H5HL_offset_into(const H5HL_t *heap, size_t offset))
/* Sanity check */
HDassert(heap);
HDassert(offset < heap->dblk_size);
if(offset >= heap->dblk_size)
H5E_THROW(H5E_CANTGET, "unable to offset into local heap data block");
ret_value = heap->dblk_image + offset;
CATCH
/* No special processing on errors */
END_FUNC(PRIV) /* end H5HL_offset_into() */
/*-------------------------------------------------------------------------
* Function: H5HL_unprotect

View File

@ -67,17 +67,17 @@ const H5O_msg_class_t H5O_MSG_EFL[1] = {{
/*-------------------------------------------------------------------------
* Function: H5O_efl_decode
* Function: H5O_efl_decode
*
* Purpose: Decode an external file list message and return a pointer to
* the message (and some other data).
* the message (and some other data).
*
* Return: Success: Ptr to a new message struct.
* Return: Success: Ptr to a new message struct.
*
* Failure: NULL
* Failure: NULL
*
* Programmer: Robb Matzke
* Tuesday, November 25, 1997
* Tuesday, November 25, 1997
*
* Modification:
* Raymond Lu
@ -90,12 +90,12 @@ static void *
H5O_efl_decode(H5F_t *f, hid_t dxpl_id, H5O_t H5_ATTR_UNUSED *open_oh,
unsigned H5_ATTR_UNUSED mesg_flags, unsigned H5_ATTR_UNUSED *ioflags, const uint8_t *p)
{
H5O_efl_t *mesg = NULL;
int version;
const char *s = NULL;
H5HL_t *heap;
size_t u; /* Local index variable */
void *ret_value = NULL; /* Return value */
H5O_efl_t *mesg = NULL;
int version;
const char *s = NULL;
H5HL_t *heap;
size_t u; /* Local index variable */
void *ret_value = NULL; /* Return value */
FUNC_ENTER_NOAPI_NOINIT
@ -104,12 +104,12 @@ H5O_efl_decode(H5F_t *f, hid_t dxpl_id, H5O_t H5_ATTR_UNUSED *open_oh,
HDassert(p);
if(NULL == (mesg = (H5O_efl_t *)H5MM_calloc(sizeof(H5O_efl_t))))
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")
/* Version */
version = *p++;
if(version != H5O_EFL_VERSION)
HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, NULL, "bad version number for external file list message")
HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, NULL, "bad version number for external file list message")
/* Reserved */
p += 3;
@ -141,24 +141,25 @@ H5O_efl_decode(H5F_t *f, hid_t dxpl_id, H5O_t H5_ATTR_UNUSED *open_oh,
/* Decode the file list */
mesg->slot = (H5O_efl_entry_t *)H5MM_calloc(mesg->nalloc * sizeof(H5O_efl_entry_t));
if(NULL == mesg->slot)
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")
if(NULL == (heap = H5HL_protect(f, dxpl_id, mesg->heap_addr, H5AC__READ_ONLY_FLAG)))
HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, NULL, "unable to read protect link value")
for(u = 0; u < mesg->nused; u++) {
/* Name */
H5F_DECODE_LENGTH (f, p, mesg->slot[u].name_offset);
/* Name */
H5F_DECODE_LENGTH (f, p, mesg->slot[u].name_offset);
s = (const char *)H5HL_offset_into(heap, mesg->slot[u].name_offset);
HDassert(s && *s);
mesg->slot[u].name = H5MM_xstrdup (s);
if((s = (const char *)H5HL_offset_into(heap, mesg->slot[u].name_offset)) == NULL)
HGOTO_ERROR(H5E_SYM, H5E_CANTGET, NULL, "unable to get external file name")
HDassert(s && *s);
mesg->slot[u].name = H5MM_xstrdup (s);
HDassert(mesg->slot[u].name);
/* File offset */
H5F_DECODE_LENGTH (f, p, mesg->slot[u].offset);
/* File offset */
H5F_DECODE_LENGTH (f, p, mesg->slot[u].offset);
/* Size */
H5F_DECODE_LENGTH (f, p, mesg->slot[u].size);
/* Size */
H5F_DECODE_LENGTH (f, p, mesg->slot[u].size);
} /* end for */
if(H5HL_unprotect(heap) < 0)

View File

@ -183,6 +183,7 @@ endforeach ()
set (HDF5_REFERENCE_TEST_FILES
aggr.h5
bad_compound.h5
bad_offset.h5
be_data.h5
be_extlink1.h5
be_extlink2.h5
@ -1336,6 +1337,7 @@ if (HDF5_BUILD_GENERATORS)
# generator executables
set (H5_GENERATORS
gen_bad_offset
gen_bad_ohdr
gen_bogus
gen_cross

View File

@ -94,7 +94,7 @@ endif
BUILD_ALL_PROGS=gen_bad_ohdr gen_bogus gen_cross gen_deflate gen_filters gen_new_array \
gen_new_fill gen_new_group gen_new_mtime gen_new_super gen_noencoder \
gen_nullspace gen_udlinks space_overflow gen_filespace gen_specmetaread \
gen_sizes_lheap gen_file_image gen_plist
gen_sizes_lheap gen_file_image gen_plist gen_bad_offset
if BUILD_ALL_CONDITIONAL
noinst_PROGRAMS=$(BUILD_ALL_PROGS)

BIN
test/bad_offset.h5 Normal file

Binary file not shown.

112
test/gen_bad_offset.c Normal file
View File

@ -0,0 +1,112 @@
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
* Copyright by The HDF Group. *
* Copyright by the Board of Trustees of the University of Illinois. *
* All rights reserved. *
* *
* This file is part of HDF5. The full HDF5 copyright notice, including *
* terms governing use, modification, and redistribution, is contained in *
* the COPYING file, which can be found at the root of the source code *
* distribution tree, or in https://support.hdfgroup.org/ftp/HDF5/releases. *
* If you do not have access to either file, you may request a copy from *
* help@hdfgroup.org. *
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
/*
* Purpose: Generate an HDF5 file for testing H5FFV-10216
*/
#include "h5test.h"
#define TESTFILE "bad_offset.h5"
#define GRP1 "group1"
#define GRP2 "group2"
#define DSET "dsetA"
#define SOFT1 "soft_one"
#define SOFT2 "soft_two"
/*-------------------------------------------------------------------------
* Function: main
* Generate an HDF5 file with groups, datasets and symbolic links.
*
* After this file is generated, write bad offset values to
* the heap at 3 locations in the file:
* (A) Open the file:
* fd = HDopen(TESTFILE, O_RDWR, 0663);
* (B) Position the file at:
* (1) HDlseek(fd, (HDoff_t)880, SEEK_SET);
* "/group1/group2": replace heap offset "8" by bad offset
* (2) HDlseek(fd, (HDoff_t)1512, SEEK_SET);
* "/dsetA": replace name offset into private heap "72" by bad offset
* (3) HDlseek(fd, (HDoff_t)1616, SEEK_SET);
* /soft_one: replace link value offset in the scratch pad "32" by bad offset
* (C) Write the bad offset value to the file:
* write(fd, &val, sizeof(val));
*
* Note: if the groups/datasets/symbolic links are changed in the file,
* the above locations need to be adjusted accordingly.
*
* Return: EXIT_SUCCESS/EXIT_FAILURE
*
*-------------------------------------------------------------------------
*/
int
main(void)
{
hid_t fid = -1, gid1 = -1, gid2 = -1; /* File and group IDs */
hid_t did = -1, sid = -1; /* Dataset and dataspace IDs */
/* Create the test file */
if((fid = H5Fcreate(TESTFILE, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT)) < 0)
FAIL_STACK_ERROR
/* Create two groups */
if((gid1 = H5Gcreate2(fid, GRP1, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0)
FAIL_STACK_ERROR
if((gid2 = H5Gcreate2(gid1, GRP2, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0)
FAIL_STACK_ERROR
/* Close the groups */
if(H5Gclose(gid1) < 0)
FAIL_STACK_ERROR
if(H5Gclose(gid2) < 0)
FAIL_STACK_ERROR
/* Create soft links to the groups */
if(H5Lcreate_soft("/group1", fid, SOFT1, H5P_DEFAULT, H5P_DEFAULT) < 0)
FAIL_STACK_ERROR
if(H5Lcreate_soft("/group1/group2", fid, SOFT2, H5P_DEFAULT, H5P_DEFAULT) < 0)
FAIL_STACK_ERROR
/* Create a dataset */
if((sid = H5Screate(H5S_SCALAR)) < 0)
FAIL_STACK_ERROR
if((did = H5Dcreate2(fid, DSET, H5T_NATIVE_INT, sid, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0)
FAIL_STACK_ERROR
/* Close the dataset */
if(H5Dclose(did) < 0)
FAIL_STACK_ERROR
/* Close the dataspace */
if(H5Sclose(sid) < 0)
FAIL_STACK_ERROR
/* Close the file */
if(H5Fclose(fid) < 0)
FAIL_STACK_ERROR
return EXIT_SUCCESS;
error:
H5E_BEGIN_TRY {
H5Gclose(gid1);
H5Gclose(gid2);
H5Dclose(did);
H5Sclose(sid);
H5Fclose(fid);
} H5E_END_TRY;
return EXIT_FAILURE;
} /* end main() */

View File

@ -318,6 +318,11 @@ typedef struct
#define MISC31_PROPNAME "misc31_prop"
#define MISC31_DTYPENAME "dtype"
/* Definitions for misc. test #33 */
/* Note that this test file is generated by "gen_bad_offset.c" */
/* and bad offset values are written to that file for testing */
#define MISC33_FILE "bad_offset.h5"
/****************************************************************
**
** test_misc1(): test unlinking a dataset from a group and immediately
@ -5472,6 +5477,55 @@ test_misc32(void)
} /* end test_misc32() */
/****************************************************************
**
** test_misc33(): Test for H5FFV-10216
** --verify that H5HL_offset_into() returns error if the
** input parameter "offset" exceeds heap data block size.
** --case (1), (2), (3) are scenarios that will traverse to the
** the 3 locations in the file having bad offset values to
** the heap. (See description in gen_bad_offset.c)
**
****************************************************************/
static void
test_misc33(void)
{
hid_t fid = -1; /* File ID */
const char *testfile = H5_get_srcdir_filename(MISC33_FILE); /* Corrected test file name */
H5O_info_t oinfo; /* Structure for object metadata information */
herr_t ret; /* Generic return value */
/* Output message about test being performed */
MESSAGE(5, ("Testing that bad offset into the heap returns error"));
/* Open the test file */
fid = H5Fopen(testfile, H5F_ACC_RDWR, H5P_DEFAULT);
CHECK(fid, FAIL, "H5Fopen");
/* Case (1) */
H5E_BEGIN_TRY {
ret = H5Oget_info_by_name(fid, "/soft_two", &oinfo, H5P_DEFAULT);
} H5E_END_TRY;
VERIFY(ret, FAIL, "H5Oget_info_by_name");
/* Case (2) */
H5E_BEGIN_TRY {
ret = H5Oget_info_by_name(fid, "/dsetA", &oinfo, H5P_DEFAULT);
} H5E_END_TRY;
VERIFY(ret, FAIL, "H5Oget_info_by_name");
/* Case (3) */
H5E_BEGIN_TRY {
ret = H5Oget_info_by_name(fid, "/soft_one", &oinfo, H5P_DEFAULT);
} H5E_END_TRY;
VERIFY(ret, FAIL, "H5Oget_info_by_name");
/* Close the file */
ret = H5Fclose(fid);
CHECK(fid, FAIL, "H5Fclose");
} /* end test_misc33() */
/****************************************************************
**
@ -5520,6 +5574,7 @@ test_misc(void)
test_misc30(); /* Exercise local heap loading bug where free lists were getting dropped */
test_misc31(); /* Test Reentering library through deprecated routines after H5close() */
test_misc32(); /* Test filter memory allocation functions */
test_misc33(); /* ??? */
} /* test_misc() */