Fix CVE-2025-6750

[bmribler]

The size of an mtime message was decoded as 0, which later caused the buffer to small resulting in a heap-buffer-overflow.

This PR fixed the issue by detecting the invalid message size and reporting the error, preventing the continuation of the code.

GH issue #5549

---

Important

Fixes CVE-2025-6750 by adding a buffer overflow check in H5O__chunk_deserialize() to prevent heap-buffer-overflow.

• Security Fix:
  • In H5O__chunk_deserialize() in H5Ocache.c, added a check to detect if mesg_size exceeds buffer end, preventing heap-buffer-overflow.
  • Removed redundant check for invalidly formatted object header message extending past end of chunk.
• Documentation:
  • Updated RELEASE.txt to include fix for CVE-2025-6750 and reference GitHub issue [BUG] A heap-buffer-overflow in H5O__mtime_new_encode at src/H5Omtime.c:237:10 #5549.

^{This description was created by for ddf0d35. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Good fix: explicitly validating that mtime messages have a non‐zero size prevents the overflow. Consider adding braces to the nested if for clarity.

[jhendersonHDF]

It may make more sense to use the H5_IS_BUFFER_OVERFLOW macro here, as in:

if (H5_IS_BUFFER_OVERFLOW(chunk_image, mesg_size, p_end))
    error;

as it does the same thing and accounts for the fact that you need p_end - chunk_image + 1. (Consider if mesg_size is 1 and p_end == chunk_image)

[fortnern]

I don't think there should be a specific case for mtime messages here. IF we really want to detect invalid messages we should create a callback for checking them, though I'm not sure we'll be able to detect all problems without fully decoding them, potentially adding unnecessary overhead.

There is a check for buffer overflow in H5O__mtime_new_decode(), why does this not catch the problem before an invalid access?

[bmribler]

@fortnern The H5O__mtime_new_decode() has the buffer overflow checks but the H5O__mtime_new_encode() doesn't and the issue #5549 is in the encoding. Would you like me to add buffer overflow checks in the encoder? I haven't seen an encoder in the library doing that although I didn't look thoroughly. The reason I added the check here is that's where the decoding of mtime is. Didn't we agree that we'll try validate the inputs where they are decoded? All that aside, just let me know what I should do now for this release. Your call.

[fortnern]

The actual message is decoded in H5O__mtime_new_decode(). The higher level obejct header code just reads the raw bytes for the messages. The message must have been decoded at some point right? Why did it not trip the error condition when it was originally decoded?

[fortnern]

It seems likely that the problem was with the off by one error in the check I mentioned in the other comment. I suspect things will work fine if you remove this one (if (id == H5O_MTIME_NEW_ID)), remove the one I mentioned (Try to detect invalidly formatted object header message that extends past end of chunk.), and implement Jordan's suggestions for the last one.

[bmribler]

I removed the "Try to..." and replaced "if (mesg_size > (size_t)(p_end - chunk_image))" with if H5_IS_BUFFER_OVERFLOW.
Next, I removed the check if (mesg_size == 0) as you suggested, the program crashed again with

WRITE of size 1 at 0x604000001d00 thread T0

in H5O__mtime_new_encode() because the buffer going into H5O__mtime_new_encode() has size 0. Understandably, the buffer overflow check serves when mesg_size is larger than expected but it doesn't help when the mesg_size is 0. That means mesg_size=0 has to be detected somewhere, right?

So, either I have to check for non-zero size in H5O__mtime_new_encode() or here
if (id == H5O_MTIME_NEW_ID) /* validate mtime message size */
if (mesg_size == 0)
HGOTO_ERROR(H5E_OHDR, H5E_BADVALUE, FAIL, "invalid size for mtime message");
Am I missing something?

[fortnern]

Yes, please try to figure out why the check in H5O__mtime_new_decode() isn't catching it.

[fortnern]

Checking for size==0 isn't a real fix because it would still fail if the size is nonzero but too small. Also the same problem could happen with other message types, each of which have their own size requirements. There should never be a decoded message in memory where the size allocated in the object header (if any) is inconsistent with the size requirements of the message.

[fortnern]

It would make sense to add an assert to H5O__mtime_encode that p_size is large enough

[fortnern]

I believe the later check for "Try to detect invalidly formatted object header message that extends past end of chunk." is now obsolete (and it had an off by one error that may have caused the problem in the first place) and can be deleted. I also think having both p_end and eom_ptr doing almost the same thing is confusing and redundant but it doesn't need to be fixed now.