File Validate-location-offset-of-the-accumulated-metadata-when-comparing.patch of Package hdf5
From: Egbert Eich <eich@suse.com>
Date: Mon Oct 10 08:43:44 2022 +0200
Subject: Validate location (offset) of the accumulated metadata when comparing
Patch-mainline: Not yet
Git-repo: ssh://eich@192.168.122.1:/home/eich/sources/HPC/hdf5
Git-commit: 2cf9918ae66f023a2b6d44eb591ee2ac479a6e53
References:
Initially, the accumulated metadata location is initialized to HADDR_UNDEF
- the highest available address. Bogus input files may provide a location
or size matching this value. Comparing this address against such bogus
values may provide false positives. This make sure, the value has been
initilized or fail the comparison early and let other parts of the
code deal with the bogus address/size.
Note: To avoid unnecessary checks, we have assumed that if the 'dirty'
member in the same structure is true the location is valid.
This fixes CVE-2018-13867.
Signed-off-by: Egbert Eich <eich@suse.com>
Signed-off-by: Egbert Eich <eich@suse.de>
---
src/H5Faccum.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
Index: hdf5-1.12.3/src/H5Faccum.c
===================================================================
--- hdf5-1.12.3.orig/src/H5Faccum.c
+++ hdf5-1.12.3/src/H5Faccum.c
@@ -47,6 +47,7 @@
#define H5F_ACCUM_THROTTLE 8
#define H5F_ACCUM_THRESHOLD 2048
#define H5F_ACCUM_MAX_SIZE (1024 * 1024) /* Max. accum. buf size (max. I/Os will be 1/2 this size) */
+#define H5F_LOC_VALID(x) (x != HADDR_UNDEF)
/******************/
/* Local Typedefs */
@@ -125,8 +126,9 @@ H5F__accum_read(H5F_shared_t *f_sh, H5FD
HDassert(!accum->buf || (accum->alloc_size >= accum->size));
/* Current read adjoins or overlaps with metadata accumulator */
- if (H5F_addr_overlap(addr, size, accum->loc, accum->size) || ((addr + size) == accum->loc) ||
- (accum->loc + accum->size) == addr) {
+ if (H5F_LOC_VALID(accum->loc) &&
+ (H5F_addr_overlap(addr, size, accum->loc, accum->size) || ((addr + size) == accum->loc) ||
+ (accum->loc + accum->size) == addr)) {
size_t amount_before; /* Amount to read before current accumulator */
haddr_t new_addr; /* New address of the accumulator buffer */
size_t new_size; /* New size of the accumulator buffer */
@@ -438,7 +440,8 @@ H5F__accum_write(H5F_shared_t *f_sh, H5F
/* Check if there is already metadata in the accumulator */
if (accum->size > 0) {
/* Check if the new metadata adjoins the beginning of the current accumulator */
- if ((addr + size) == accum->loc) {
+ if (H5F_LOC_VALID(accum->loc)
+ && (addr + size) == accum->loc) {
/* Check if we need to adjust accumulator size */
if (H5F__accum_adjust(accum, file, H5F_ACCUM_PREPEND, size) < 0)
HGOTO_ERROR(H5E_IO, H5E_CANTRESIZE, FAIL, "can't adjust metadata accumulator")
@@ -463,7 +466,8 @@ H5F__accum_write(H5F_shared_t *f_sh, H5F
accum->dirty_off = 0;
} /* end if */
/* Check if the new metadata adjoins the end of the current accumulator */
- else if (addr == (accum->loc + accum->size)) {
+ else if (H5F_LOC_VALID(accum->loc) &&
+ addr == (accum->loc + accum->size)) {
/* Check if we need to adjust accumulator size */
if (H5F__accum_adjust(accum, file, H5F_ACCUM_APPEND, size) < 0)
HGOTO_ERROR(H5E_IO, H5E_CANTRESIZE, FAIL, "can't adjust metadata accumulator")
@@ -484,7 +488,8 @@ H5F__accum_write(H5F_shared_t *f_sh, H5F
accum->size += size;
} /* end if */
/* Check if the piece of metadata being written overlaps the metadata accumulator */
- else if (H5F_addr_overlap(addr, size, accum->loc, accum->size)) {
+ else if (H5F_LOC_VALID(accum->loc) &&
+ H5F_addr_overlap(addr, size, accum->loc, accum->size)) {
size_t add_size; /* New size of the accumulator buffer */
/* Check if the new metadata is entirely within the current accumulator */
@@ -744,7 +749,8 @@ H5F__accum_write(H5F_shared_t *f_sh, H5F
/* (Note that this could be improved by updating the accumulator
* with [some of] the information just read in. -QAK)
*/
- if (H5F_addr_overlap(addr, size, accum->loc, accum->size)) {
+ if (H5F_LOC_VALID(accum->loc) &&
+ H5F_addr_overlap(addr, size, accum->loc, accum->size)) {
/* Check for write starting before beginning of accumulator */
if (H5F_addr_le(addr, accum->loc)) {
/* Check for write ending within accumulator */
@@ -867,6 +873,7 @@ H5F__accum_free(H5F_shared_t *f_sh, H5FD
/* Adjust the metadata accumulator to remove the freed block, if it overlaps */
if ((f_sh->feature_flags & H5FD_FEAT_ACCUMULATE_METADATA) &&
+ H5F_LOC_VALID(accum->loc) &&
H5F_addr_overlap(addr, size, accum->loc, accum->size)) {
size_t overlap_size; /* Size of overlap with accumulator */
