Skip to content

Commit 09e3cfe

Browse files
authored
Add buffer overflow detection (#5884)
Add buffer overflow checks to H5C__decode_cache_image_header() in H5Cimage.c for security hardening. Security Hardening: Add buffer overflow checks in H5C__decode_cache_image_header() in H5Cimage.c. Use H5_IS_BUFFER_OVERFLOW() to verify buffer space before accessing elements. Checks added for signature, version, flags, image data length, and number of entries. Misc: Adjust H5C__reconstruct_cache_contents() to use updated buffer handling logic.
1 parent 849de4e commit 09e3cfe

File tree

1 file changed

+15
-6
lines changed

1 file changed

+15
-6
lines changed

src/H5Cimage.c

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1277,7 +1277,8 @@ H5C__decode_cache_image_header(const H5F_t *f, H5C_t *cache_ptr, const uint8_t *
12771277
size_t actual_header_len;
12781278
size_t expected_header_len;
12791279
const uint8_t *p;
1280-
herr_t ret_value = SUCCEED; /* Return value */
1280+
const uint8_t *p_end = *buf + buf_size - 1; /* End of the p buffer */
1281+
herr_t ret_value = SUCCEED; /* Return value */
12811282

12821283
FUNC_ENTER_PACKAGE
12831284

@@ -1290,7 +1291,7 @@ H5C__decode_cache_image_header(const H5F_t *f, H5C_t *cache_ptr, const uint8_t *
12901291
p = *buf;
12911292

12921293
/* Ensure buffer has enough data for signature comparison */
1293-
if (H5_IS_BUFFER_OVERFLOW(p, H5C__MDCI_BLOCK_SIGNATURE_LEN, *buf + buf_size - 1))
1294+
if (H5_IS_BUFFER_OVERFLOW(p, H5C__MDCI_BLOCK_SIGNATURE_LEN, p_end))
12941295
HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, FAIL, "Insufficient buffer size for signature");
12951296

12961297
/* Check signature */
@@ -1299,25 +1300,33 @@ H5C__decode_cache_image_header(const H5F_t *f, H5C_t *cache_ptr, const uint8_t *
12991300
p += H5C__MDCI_BLOCK_SIGNATURE_LEN;
13001301

13011302
/* Check version */
1303+
if (H5_IS_BUFFER_OVERFLOW(p, 1, p_end))
1304+
HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
13021305
version = *p++;
13031306
if (version != (uint8_t)H5C__MDCI_BLOCK_VERSION_0)
13041307
HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, FAIL, "Bad metadata cache image version");
13051308

13061309
/* Decode flags */
1310+
if (H5_IS_BUFFER_OVERFLOW(p, 1, p_end))
1311+
HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
13071312
flags = *p++;
13081313
if (flags & H5C__MDCI_HEADER_HAVE_RESIZE_STATUS)
13091314
have_resize_status = true;
13101315
if (have_resize_status)
13111316
HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, FAIL, "MDC resize status not yet supported");
13121317

13131318
/* Read image data length */
1319+
if (H5_IS_BUFFER_OVERFLOW(p, H5F_sizeof_size(f), p_end))
1320+
HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
13141321
H5F_DECODE_LENGTH(f, p, cache_ptr->image_data_len);
13151322

13161323
/* For now -- will become <= eventually */
13171324
if (cache_ptr->image_data_len != cache_ptr->image_len)
13181325
HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, FAIL, "Bad metadata cache image data length");
13191326

13201327
/* Read num entries */
1328+
if (H5_IS_BUFFER_OVERFLOW(p, 4, p_end))
1329+
HGOTO_ERROR(H5E_CACHE, H5E_OVERFLOW, FAIL, "ran off end of input buffer while decoding");
13211330
UINT32DECODE(p, cache_ptr->num_entries_in_image);
13221331
if (cache_ptr->num_entries_in_image == 0)
13231332
HGOTO_ERROR(H5E_CACHE, H5E_BADVALUE, FAIL, "Bad metadata cache entry count");
@@ -2392,18 +2401,18 @@ H5C__reconstruct_cache_contents(H5F_t *f, H5C_t *cache_ptr)
23922401
assert(cache_ptr->image_len > 0);
23932402

23942403
/* Decode metadata cache image header */
2395-
p = (uint8_t *)cache_ptr->image_buffer;
2396-
image_len = cache_ptr->image_len;
2397-
if (H5C__decode_cache_image_header(f, cache_ptr, &p, image_len + 1) < 0)
2404+
p = (uint8_t *)cache_ptr->image_buffer;
2405+
if (H5C__decode_cache_image_header(f, cache_ptr, &p, cache_ptr->image_len + 1) < 0)
23982406
HGOTO_ERROR(H5E_CACHE, H5E_CANTDECODE, FAIL, "cache image header decode failed");
2399-
assert((size_t)(p - (uint8_t *)cache_ptr->image_buffer) < image_len);
2407+
assert((size_t)(p - (uint8_t *)cache_ptr->image_buffer) < cache_ptr->image_len);
24002408

24012409
/* The image_data_len and # of entries should be defined now */
24022410
assert(cache_ptr->image_data_len > 0);
24032411
assert(cache_ptr->image_data_len <= cache_ptr->image_len);
24042412
assert(cache_ptr->num_entries_in_image > 0);
24052413

24062414
/* Reconstruct entries in image */
2415+
image_len = cache_ptr->image_len;
24072416
for (u = 0; u < cache_ptr->num_entries_in_image; u++) {
24082417
/* Create the prefetched entry described by the ith
24092418
* entry in cache_ptr->image_entrise.

0 commit comments

Comments
 (0)