Fix CVE-2025-2926 #5841
Fix CVE-2025-2926 #5841
Conversation
bmribler
requested review from
brtnfld,
byrnHDF,
derobins,
fortnern,
glennsong09,
jhendersonHDF,
lrknox,
mattjala,
qkoziol and
vchoi-hdfgroup
as code owners
|
Umm, FYI, this PR only has the changes in H5Centry.c and RELEASE.txt. Sorry! |
release_docs/RELEASE.txt
Outdated
| =================================== | ||
| Library | ||
| ------- | ||
| - Fixed CVE 2025 2926 |
There was a problem hiding this comment.
Should use hyphens in the name, like other CVE issues
There was a problem hiding this comment.
Hmm, I don't know how that happened. I always have hyphens... Thanks, Dana.
src/H5Cimage.c
Outdated
| if (H5C__decode_cache_image_header(f, cache_ptr, &p, image_len + 1) < 0) | ||
| HGOTO_ERROR(H5E_CACHE, H5E_CANTDECODE, FAIL, "cache image header decode failed"); | ||
| assert((size_t)(p - (uint8_t *)cache_ptr->image_buffer) < cache_ptr->image_len); | ||
| assert((size_t)(p - (uint8_t *)cache_ptr->image_buffer) < image_len); |
There was a problem hiding this comment.
Why not make this a real error check instead of an assert?
There was a problem hiding this comment.
If H5C__decode_cache_image_header checks for overflow an assert is appropriate here. And if it doesn't, it should.
There was a problem hiding this comment.
This change was merged into this PR by accident and that was fixed now. I'll check out about H5C__decode_cache_image_header() and create another PR instead.
src/H5Centry.c
Outdated
| if (type->get_initial_load_size(udata, &len) < 0) | ||
| HGOTO_ERROR(H5E_CACHE, H5E_CANTGET, NULL, "can't retrieve image size"); | ||
| assert(len > 0); | ||
| if (len == 0) |
There was a problem hiding this comment.
Seems to me this should really be checked in the callbacks, and left as an assert here.
There was a problem hiding this comment.
Do you know why the callback is returning len=0 without returning an error?
There was a problem hiding this comment.
I fixed it and push the change soon
jhendersonHDF
force-pushed
the
fix_CVE-2025-2926
branch
from
a4678ff to
b36c123
Compare
github-project-automation
bot
moved this from Scheduled/On-Deck
to In progress
in HDF5 - TRIAGE & TRACK
|
I think we still need to get to the bottom of this. I did some digging and it looks like the size in the udata comes from a continuation message, but H5O__cont_decode() checks for size == 0. Can you try to figure out how it's getting past that check? |
|
@fortnern I believe this commit took care of the len=0 issue. Although, I'm not exactly sure how that commit is not listed in the changed files of #5841... |
This check isn't as close to the file as it could be. As I mentioned previously, the udata->size in that function should come from H5O__cont_decode(), which already has a check for size == 0. Can you try to figure out how that was bypassed? |
5 participants
An image size was corrupted and decoded as 0 resulting in a NULL image buffer, which caused a NULL pointer dereference when the image being copied to the buffer. This PR adds the image size check.
Fixes #5384
Important
Fixes CVE-2025-2926 by adding an image size check in
H5O__cache_chk_get_initial_load_size()to prevent NULL pointer dereference.H5O__cache_chk_get_initial_load_size()inH5Ocache.cto prevent NULL pointer dereference when image size is corrupted and decoded as 0.CHANGELOG.mdto document the fix for CVE-2025-2926.This description was created by
for 05369d0. You can customize this summary. It will automatically update as commits are pushed.