From 068fc878c39a37c0b3865cb6cd01eb57f4dbde74 Mon Sep 17 00:00:00 2001 From: Binh-Minh Ribler Date: Mon, 3 Aug 2020 12:48:58 -0500 Subject: Fix HDFFV-11120 and HDFFV-11121 (CVE-2018-13870 and CVE-2018-13869) Description: When a buffer overflow occurred because a name length was corrupted and became very large, h5dump produced a segfault on one file and a memcpy parameter overlap on another file. This commit added checks that detect a read pass the end of the buffer to prevent these error conditions. Platforms tested: Linux/64 (jelly) --- release_docs/RELEASE.txt | 11 +++++++++++ src/H5Olink.c | 19 ++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 82446ab..8a86b82 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -342,6 +342,17 @@ Bug Fixes since HDF5-1.10.5 release Library ------- + - Fixed issues CVE-2018-13870 and CVE-2018-13869 + + When a buffer overflow occurred because a name length was corrupted + and became very large, h5dump crashed on memory access violation. + + A check for reading pass the end of the buffer was added to multiple + locations to prevent the crashes and h5dump now simply fails with an + error message when this error condition occurs. + + (BMR - 2020/7/31, HDFFV-11120 and HDFFV-11121) + - Fixed the segmentation fault when reading attributes with multiple threads It was reported that the reading of attributes with variable length string diff --git a/src/H5Olink.c b/src/H5Olink.c index 10479e6..30bac65 100644 --- a/src/H5Olink.c +++ b/src/H5Olink.c @@ -119,11 +119,12 @@ H5FL_DEFINE_STATIC(H5O_link_t); static void * H5O__link_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, unsigned H5_ATTR_UNUSED mesg_flags, unsigned H5_ATTR_UNUSED *ioflags, - size_t H5_ATTR_UNUSED p_size, const uint8_t *p) + size_t p_size, const uint8_t *p) { H5O_link_t *lnk = NULL; /* Pointer to link message */ size_t len = 0; /* Length of a string in the message */ unsigned char link_flags; /* Flags for encoding link info */ + const uint8_t *p_end = p + p_size; /* End of the p buffer */ void *ret_value = NULL; /* Return value */ FUNC_ENTER_STATIC @@ -199,6 +200,11 @@ H5O__link_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, if(len == 0) HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, NULL, "invalid name length") + /* Make sure that length doesn't exceed buffer size, which could occur + when the file is corrupted */ + if(p + len > p_end) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "name length causes read past end of buffer") + /* Get the link's name */ if(NULL == (lnk->name = (char *)H5MM_malloc(len + 1))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") @@ -218,6 +224,12 @@ H5O__link_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, UINT16DECODE(p, len) if(len == 0) HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, NULL, "invalid link length") + + /* Make sure that length doesn't exceed buffer size, which could occur + when the file is corrupted */ + if(p + len > p_end) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "name length causes read past end of buffer") + if(NULL == (lnk->u.soft.name = (char *)H5MM_malloc((size_t)len + 1))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") H5MM_memcpy(lnk->u.soft.name, p, len); @@ -238,6 +250,11 @@ H5O__link_decode(H5F_t *f, H5O_t H5_ATTR_UNUSED *open_oh, lnk->u.ud.size = len; if(len > 0) { + /* Make sure that length doesn't exceed buffer size, which could + occur when the file is corrupted */ + if(p + len > p_end) + HGOTO_ERROR(H5E_OHDR, H5E_OVERFLOW, NULL, "name length causes read past end of buffer") + if(NULL == (lnk->u.ud.udata = H5MM_malloc((size_t)len))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") H5MM_memcpy(lnk->u.ud.udata, p, len); -- cgit v0.12 From a324025e00adf3dd0a9d475cfff5af2ec4c7576c Mon Sep 17 00:00:00 2001 From: Binh-Minh Ribler Date: Tue, 4 Aug 2020 12:06:45 -0500 Subject: Fixed typo --- release_docs/RELEASE.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 8a86b82..3a85dcc 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -347,7 +347,7 @@ Bug Fixes since HDF5-1.10.5 release When a buffer overflow occurred because a name length was corrupted and became very large, h5dump crashed on memory access violation. - A check for reading pass the end of the buffer was added to multiple + A check for reading past the end of the buffer was added to multiple locations to prevent the crashes and h5dump now simply fails with an error message when this error condition occurs. -- cgit v0.12