From e175d24be1f551b499045026f16f7bee79a90f99 Mon Sep 17 00:00:00 2001 From: Binh-Minh Date: Wed, 10 Jun 2026 10:47:36 -0400 Subject: [PATCH 1/3] Add new APIs to address vulnerabilities The existing APIs Vgetname, Vgetclass, and Vinquire posed multiple vulnerabilities due to the lack of size checks. Because changing their signature will break compatibilities, three new APIs are added in place of their deprecation instead. New APIs are: int Vgetname40(int32 vkey, char *vgname, size_t *buf_size); int Vgetclass40(int32 vkey, char *vgclass, size_t *buf_size); int Vinquire40(int32 vkey, int32 *nentries, char *vgname, size_t *buf_size); Fixes GH #872 --- hdf/src/hproto.h | 27 ++++- hdf/src/vgp.c | 267 ++++++++++++++++++++++++++++++++++++++++++++--- hdf/test/tvset.c | 32 ++++++ 3 files changed, 304 insertions(+), 22 deletions(-) diff --git a/hdf/src/hproto.h b/hdf/src/hproto.h index 4b080b995..c0d454690 100644 --- a/hdf/src/hproto.h +++ b/hdf/src/hproto.h @@ -1402,15 +1402,21 @@ HDFLIBAPI int32 Vgetid(HFILEID f, int32 vgid); HDFLIBAPI int32 Vgetnext(int32 vkey, int32 id); -HDFLIBAPI int32 Vgetname(int32 vkey, char *vgname); +HDFLIBAPI int Vgetname40(int32 vkey, char *vgname, size_t *buf_size); /* in 4.0 */ -HDFLIBAPI int32 Vgetnamelen(int32 vkey, uint16 *name_len); +HDFLIBAPI int Vgetclass40(int32 vkey, char *vgclass, size_t *buf_size); /* in 4.0 */ -HDFLIBAPI int32 Vgetclassnamelen(int32 vkey, uint16 *classname_len); +HDFLIBAPI int Vinquire40(int32 vkey, int32 *nentries, char *vgname, size_t *buf_size); /* in 4.0 */ -HDFLIBAPI int32 Vgetclass(int32 vkey, char *vgclass); +HDFLIBAPI int32 Vgetname(int32 vkey, char *vgname); /* deprecated in 4.0 */ -HDFLIBAPI int Vinquire(int32 vkey, int32 *nentries, char *vgname); +HDFLIBAPI int32 Vgetnamelen(int32 vkey, uint16 *name_len); /* deprecated in 4.0 */ + +HDFLIBAPI int32 Vgetclassnamelen(int32 vkey, uint16 *classname_len); /* deprecated in 4.0 */ + +HDFLIBAPI int32 Vgetclass(int32 vkey, char *vgclass); /* deprecated in 4.0 */ + +HDFLIBAPI int Vinquire(int32 vkey, int32 *nentries, char *vgname); /* deprecated in 4.0 */ HDFLIBAPI int32 Vdelete(int32 f, int32 ref); @@ -1441,6 +1447,17 @@ HDFLIBAPI int Vdeletetagref(int32 vkey, /* IN: vgroup key */ HDFLIBAPI int VPshutdown(void); +/* These are deprecated in version 4.0 */ +HDFLIBAPI int32 Vgetname(int32 vkey, char *vgname); + +HDFLIBAPI int32 Vgetnamelen(int32 vkey, uint16 *name_len); + +HDFLIBAPI int32 Vgetclassnamelen(int32 vkey, uint16 *classname_len); + +HDFLIBAPI int32 Vgetclass(int32 vkey, char *vgclass); + +HDFLIBAPI int Vinquire(int32 vkey, int32 *nentries, char *vgname); + /* ** from vparse.c */ diff --git a/hdf/src/vgp.c b/hdf/src/vgp.c index 0afc8a466..ec127922b 100644 --- a/hdf/src/vgp.c +++ b/hdf/src/vgp.c @@ -29,6 +29,7 @@ LOCAL ROUTINES New_vfile -- create new vgroup file record Load_vfile -- loads vgtab table with info of all vgroups in file. Remove_vfile -- removes the file ptr from the vfile[] table. + VIGet_vgdesc -- get and verify the vgroup VPgetinfo -- Read in the "header" information about the Vgroup. VIstart -- V-level initialization routine @@ -36,7 +37,7 @@ LOCAL ROUTINES EXPORTED ROUTINES ================= - Following 4 routines are solely for B-tree routines. + Following 5 routines are solely for B-tree routines. vcompare -- Compares two TBBT-tree keys for equality. Similar to memcmp. vprint -- Prints out the key and reference number of VDatas and Vgroups vdestroynode -- destroy vgroup node in TBBT @@ -73,19 +74,22 @@ EXPORTED ROUTINES Vaddtagref -- Inserts a tag/ref pair into the attached vgroup vg. vinsertpair -- Inserts a tag/ref pair into the attached vgroup vg. Ventries -- Returns the num of entries (+ve integer) in the vgroup vgid. - Vsetname -- Gives a name to the VGROUP vg. - Vsetclass -- Assigns a class name to the VGROUP vg. + Vsetname -- Gives a name to the Vgroup vg. + Vsetclass -- Assigns a class name to the Vgroup vg. Visvg -- Tests if the given entry in the vgroup vg is a VGROUP. Visvs -- Checks if an id in a vgroup refers to a VDATA. Vgetid -- Given a vgroup's id, returns the next vgroup's id in the file. Vgetnext -- Given the id of an entry from a vgroup vg, looks in vg for the next entry after it, and returns its id. - Vgetnamelen -- Retrieves the length of the vgroup's name. - Vgetclassnamelen -- Retrieves the length of the vgroup's classname. - Vgetname -- Returns the vgroup's name. - Vgetclass -- Returns the vgroup's class name . + Vgetnamelen -- Retrieves the length of the vgroup's name. - Deprecated + Vgetclassnamelen -- Retrieves the length of the vgroup's classname. - Deprecated + Vgetname -- Returns the vgroup's name. - Deprecated + Vgetname40 -- Returns the vgroup's name. (introduced in 4.0) + Vgetclass -- Returns the vgroup's class. - Deprecated + Vgetclass40 -- Returns the vgroup's class. (introduced in 4.0) Vgetvgroups -- Gets user-created vgroups in a file or in a vgroup - Vinquire -- General inquiry routine for VGROUP. + Vinquire -- General inquiry routine for Vgroup. - Deprecated + Vinquire40 -- General inquiry routine for Vgroup. (introduced in 4.0) Vopen -- This routine opens the HDF file and initializes it for Vset operations.(i.e." Hopen(); Vinitialize(f)"). Vclose -- This routine closes the HDF file, after it has freed @@ -96,9 +100,6 @@ EXPORTED ROUTINES as well as from the file. Vdeletetagref - delete tag/ref pair in Vgroup - NOTE: Another pass needs to made through this file to update some of - the comments about certain sections of the code. -GV 9/8/97 - *************************************************************************/ #include "hdf_priv.h" @@ -372,7 +373,7 @@ Load_vfile(HFILEID f /* IN: file handle */) ret = aid = Hstartread(f, DFTAG_VG, DFREF_WILDCARD); while (ret != FAIL) { - /* get tag/ref for this vgroup */ + /* get tag/ref for this Vgroup */ HQuerytagref(aid, &tag, &ref); /* get a vgroup struct to fill */ @@ -2405,14 +2406,13 @@ Vgetnext(int32 vkey, /* IN: vgroup key */ /******************************************************************************* NAME - Vgetnamelen + Vgetnamelen - Deprecated in favor of Vgetname40 DESCRIPTION Retrieves the length of the vgroup's name. RETURNS Returns SUCCEED/FAIL - BMR - 2006/09/10 *******************************************************************************/ int32 @@ -2456,14 +2456,13 @@ Vgetnamelen(int32 vkey, /* IN: vgroup key */ /******************************************************************************* NAME - Vgetclassnamelen + Vgetclassnamelen - Deprecated in favor of Vgetclass40 DESCRIPTION Retrieves the length of the vgroup's name. RETURNS Returns SUCCEED/FAIL - BMR - 2006/09/10 *******************************************************************************/ int32 @@ -2503,7 +2502,7 @@ Vgetclassnamelen(int32 vkey, /* IN: vgroup key */ /******************************************************************************* NAME - Vgetname + Vgetname - Deprecated in favor of Vgetname40 DESCRIPTION returns the vgroup's name @@ -3169,3 +3168,237 @@ Vgetvgroups(int32 id, /* IN: file id or vgroup id */ done: return ret_value; } /* Vgetvgroups */ + +/****************************************************************************** + * Introduced in 4.0 to address security issues * + ******************************************************************************/ + +/******************************************************************************* + NAME + VIget_vgdesc -- get and verify the vgroup + + DESCRIPTION + Return a pointer to a VGROUP for subsequent accesses of the vgroup. + + RETURNS + VGROUP record pointer or NULL if failed. + +*******************************************************************************/ +VGROUP * +VIGet_vgdesc(int32 vkey /* IN: vgroup key */) +{ + + vginstance_t *v = NULL; + VGROUP *vg = NULL; + VGROUP *ret_value = NULL; + + /* get and verify the vgroup */ + if (NULL == (v = (vginstance_t *)HAatom_object(vkey))) + HGOTO_ERROR(DFE_NOVS, NULL); + vg = v->vg; + if (vg == NULL) + HGOTO_ERROR(DFE_BADPTR, NULL); + if (vg->otag != DFTAG_VG) + HGOTO_ERROR(DFE_ARGS, NULL); + + ret_value = vg; +done: + return ret_value; +} + +/******************************************************************************* +NAME + Vgetname40 - Retrieves the vgroup's name + +DESCRIPTION + This function retrieves the name of a vgroup. buf_size is an IN/OUT + parameter. When vgname is NULL or *buf_size is 0, the function returns + the length of the vgroup name in *buf_size without copying. Otherwise, + up to *buf_size-1 characters are stored in vgname followed by a null + terminator, and the actual name length is returned in *buf_size. If the + name of the vgroup is longer than *buf_size-1, the string will be truncated + and the null terminator is stored in the last position of the buffer. + buf_size must not be NULL. + +RETURNS + SUCCEED / FAIL + +*******************************************************************************/ +int +Vgetname40(int32 vkey, /* IN: vgroup key */ + char *vgname, /* OUT: vgroup name */ + size_t *buf_size /* IN/OUT: name buffer size */) +{ + VGROUP *vg = NULL; + size_t name_len = 0; + int ret_value = SUCCEED; + + /* Clear error stack */ + HEclear(); + + /* Check arguments */ + if (HAatom_group(vkey) != VGIDGROUP || buf_size == NULL) + HGOTO_ERROR(DFE_ARGS, FAIL); + + /* Get the vgroup struct for access */ + if ((vg = VIGet_vgdesc(vkey)) == NULL) + HGOTO_ERROR(DFE_BADPTR, FAIL); + + /* Get the length of the vgroup name */ + name_len = (vg->vgname != NULL) ? strlen(vg->vgname) : 0; + + /* If vgname is NULL or *buf_size is 0, return the length of the name */ + if (vgname == NULL || *buf_size == 0) { + *buf_size = name_len; + HGOTO_DONE(ret_value); + } + + /* Copy vgroup name, truncating if necessary */ + if (vg->vgname != NULL) { + strncpy(vgname, vg->vgname, *buf_size - 1); + vgname[*buf_size - 1] = '\0'; + } + else + vgname[0] = '\0'; + + /* Return the actual name length */ + *buf_size = name_len; + +done: + return ret_value; +} /* Vgetname40 */ + +/******************************************************************************* +NAME + Vgetclass40 - Retrieves the vgroup's class + +DESCRIPTION + This function retrieves the class of a vgroup. buf_size is an IN/OUT + parameter. When vgclass is NULL or *buf_size is 0, the function returns + the length of the vgroup class in *buf_size without copying. Otherwise, + up to *buf_size-1 characters are stored in vgclass followed by a null + terminator, and the actual class length is returned in *buf_size. If the + class of the vgroup is longer than *buf_size-1, the string will be truncated + and the null terminator is stored in the last position of the buffer. + buf_size must not be NULL. + +RETURNS + SUCCEED / FAIL + +*******************************************************************************/ +int +Vgetclass40(int32 vkey, /* IN: vgroup key */ + char *vgclass, /* OUT: vgroup class buffer */ + size_t *buf_size /* IN/OUT: class buffer size */) +{ + VGROUP *vg = NULL; + size_t class_len = 0; + int ret_value = SUCCEED; + + /* Clear error stack */ + HEclear(); + + /* Check arguments */ + if (HAatom_group(vkey) != VGIDGROUP || buf_size == NULL) + HGOTO_ERROR(DFE_ARGS, FAIL); + + /* Get the vgroup struct for access */ + if ((vg = VIGet_vgdesc(vkey)) == NULL) + HGOTO_ERROR(DFE_BADPTR, FAIL); + + /* Get the length of the vgroup class */ + class_len = (vg->vgclass != NULL) ? strlen(vg->vgclass) : 0; + + /* If vgclass is NULL or *buf_size is 0, return the length of the class */ + if (vgclass == NULL || *buf_size == 0) { + *buf_size = class_len; + HGOTO_DONE(ret_value); + } + + /* Copy vgroup class, truncating if necessary */ + if (vg->vgclass != NULL) { + strncpy(vgclass, vg->vgclass, *buf_size - 1); + vgclass[*buf_size - 1] = '\0'; + } + else + vgclass[0] = '\0'; + + /* Return the actual class length */ + *buf_size = class_len; + +done: + return ret_value; +} /* Vgetclass40 */ + +/******************************************************************************* +NAME + Vinquire40 + +DESCRIPTION + General inquiry routine for VGROUP (introduced in 4.0) + + nentries - OUT: buffer for number of entries in the vgroup + buf_size - IN/OUT: size of the vgname buffer on input; actual length + of the vgroup name on output. If vgname is NULL or + *buf_size is 0, the function returns the length of the + vgroup name in *buf_size without copying. Otherwise, up + to *buf_size-1 characters are stored in vgname followed + by a null terminator, and the actual name length is + returned in *buf_size. If the name is longer than + *buf_size-1, the string is truncated and the null + terminator is stored in the last position of the buffer. + If buf_size is NULL, the vgroup name is not retrieved. + vgname - OUT: the vgroup's name, set if non-NULL and *buf_size > 0 + +RETURNS + SUCCEED/FAIL + +*******************************************************************************/ +int +Vinquire40(int32 vkey, /* IN: vgroup key */ + int32 *nentries, /* OUT: number of entries in vgroup */ + char *vgname, /* OUT: vgroup name */ + size_t *buf_size /* IN/OUT: vgname size */) +{ + VGROUP *vg = NULL; + size_t name_len = 0; + int ret_value = SUCCEED; + + /* Clear error stack */ + HEclear(); + + /* Check argument */ + if (HAatom_group(vkey) != VGIDGROUP) + HGOTO_ERROR(DFE_ARGS, FAIL); + + /* Get the vgroup struct for access */ + if ((vg = VIGet_vgdesc(vkey)) == NULL) + HGOTO_ERROR(DFE_BADPTR, FAIL); + + /* Get the length of the vgroup name */ + name_len = (vg->vgname != NULL) ? strlen(vg->vgname) : 0; + + /* Get the number of entries in vgroup, if requested */ + if (nentries != NULL) + *nentries = (int32)vg->nvelt; + + /* If buf_size is provided, handle name copy or length query */ + if (buf_size != NULL) { + if (vgname == NULL || *buf_size == 0) { + *buf_size = name_len; /* return the name length */ + } + else { + /* Copy vgroup name, truncating if necessary */ + if (vg->vgname != NULL) { + strncpy(vgname, vg->vgname, *buf_size - 1); + vgname[*buf_size - 1] = '\0'; + } + else + vgname[0] = '\0'; + *buf_size = name_len; /* return the name length */ + } + } + +done: + return ret_value; +} /* Vinquire40 */ diff --git a/hdf/test/tvset.c b/hdf/test/tvset.c index 608681397..3d20aabdf 100644 --- a/hdf/test/tvset.c +++ b/hdf/test/tvset.c @@ -563,10 +563,12 @@ read_vset_stuff(void) int32 vg1; int32 vs1; int32 status, num, i, count, intr, sz; + int statusint = SUCCEED; float32 fl_expected; int32 in_expected; char8 c_expected; uint16 name_len; + size_t buf_size; ibuf = (int32 *)malloc(sizeof(float32) * 2000); fbuf = (float32 *)malloc(sizeof(float32) * 2000); @@ -603,6 +605,35 @@ read_vset_stuff(void) printf(">>> Was not able to attach (r) Vgroup %d\n", (int)list[0]); } + /* get the vgroup name's length, allocate the buffer, then get the vgroup name */ + statusint = Vgetname40(vg1, NULL, &buf_size); + CHECK(statusint, FAIL, "Vgetname40:vg1"); + + vgname = (char *)malloc(sizeof(char) * (buf_size + 1)); + CHECK_ALLOC(vgname, "vgname", "read_vset_stuff"); + + statusint = Vgetname40(vg1, vgname, &buf_size); + CHECK(statusint, FAIL, "Vgetname40:vg1"); + + fprintf(stderr, "Vgetname40 returns name = %s\n", vgname); + free(vgname); + + /* get the vgroup class' length, allocate the buffer, then get the vgroup class */ + statusint = Vgetclass40(vg1, NULL, &buf_size); + CHECK(statusint, FAIL, "Vgetclass40:vg1"); + + vgclass = (char *)malloc(sizeof(char) * (buf_size + 1)); + CHECK_ALLOC(vgclass, "vgclass", "read_vset_stuff"); + + statusint = Vgetclass40(vg1, vgclass, &buf_size); + CHECK(statusint, FAIL, "Vgetclass40:vg1"); + + free(vgclass); + + /* get the vgroup name with insufficient buffer space */ + + /* Tests deprecated functions */ + status = Vgetnamelen(vg1, &name_len); CHECK(status, FAIL, "Vgetnamelen:vg1"); @@ -611,6 +642,7 @@ read_vset_stuff(void) status = Vgetname(vg1, vgname); CHECK(status, FAIL, "Vgetname:vg1"); +fprintf(stderr, "Vgetname returns name = %s\n", vgname); status = Vgetclassnamelen(vg1, &name_len); CHECK(status, FAIL, "Vgetclassnamelen:vg1"); From 973674fa9ecb72239369c4e5a7610125a4078104 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 10 Jun 2026 15:08:39 +0000 Subject: [PATCH 2/3] Committing clang-format changes --- hdf/src/vgp.c | 26 +++++++++++++------------- hdf/test/tvset.c | 4 ++-- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/hdf/src/vgp.c b/hdf/src/vgp.c index ec127922b..eba82491d 100644 --- a/hdf/src/vgp.c +++ b/hdf/src/vgp.c @@ -3225,9 +3225,9 @@ RETURNS *******************************************************************************/ int -Vgetname40(int32 vkey, /* IN: vgroup key */ - char *vgname, /* OUT: vgroup name */ - size_t *buf_size /* IN/OUT: name buffer size */) +Vgetname40(int32 vkey, /* IN: vgroup key */ + char *vgname, /* OUT: vgroup name */ + size_t *buf_size /* IN/OUT: name buffer size */) { VGROUP *vg = NULL; size_t name_len = 0; @@ -3287,13 +3287,13 @@ RETURNS *******************************************************************************/ int -Vgetclass40(int32 vkey, /* IN: vgroup key */ - char *vgclass, /* OUT: vgroup class buffer */ - size_t *buf_size /* IN/OUT: class buffer size */) +Vgetclass40(int32 vkey, /* IN: vgroup key */ + char *vgclass, /* OUT: vgroup class buffer */ + size_t *buf_size /* IN/OUT: class buffer size */) { - VGROUP *vg = NULL; - size_t class_len = 0; - int ret_value = SUCCEED; + VGROUP *vg = NULL; + size_t class_len = 0; + int ret_value = SUCCEED; /* Clear error stack */ HEclear(); @@ -3355,10 +3355,10 @@ RETURNS *******************************************************************************/ int -Vinquire40(int32 vkey, /* IN: vgroup key */ - int32 *nentries, /* OUT: number of entries in vgroup */ - char *vgname, /* OUT: vgroup name */ - size_t *buf_size /* IN/OUT: vgname size */) +Vinquire40(int32 vkey, /* IN: vgroup key */ + int32 *nentries, /* OUT: number of entries in vgroup */ + char *vgname, /* OUT: vgroup name */ + size_t *buf_size /* IN/OUT: vgname size */) { VGROUP *vg = NULL; size_t name_len = 0; diff --git a/hdf/test/tvset.c b/hdf/test/tvset.c index 3d20aabdf..38fa7c37d 100644 --- a/hdf/test/tvset.c +++ b/hdf/test/tvset.c @@ -615,7 +615,7 @@ read_vset_stuff(void) statusint = Vgetname40(vg1, vgname, &buf_size); CHECK(statusint, FAIL, "Vgetname40:vg1"); - fprintf(stderr, "Vgetname40 returns name = %s\n", vgname); + fprintf(stderr, "Vgetname40 returns name = %s\n", vgname); free(vgname); /* get the vgroup class' length, allocate the buffer, then get the vgroup class */ @@ -642,7 +642,7 @@ read_vset_stuff(void) status = Vgetname(vg1, vgname); CHECK(status, FAIL, "Vgetname:vg1"); -fprintf(stderr, "Vgetname returns name = %s\n", vgname); + fprintf(stderr, "Vgetname returns name = %s\n", vgname); status = Vgetclassnamelen(vg1, &name_len); CHECK(status, FAIL, "Vgetclassnamelen:vg1"); From 79e26d8eefd7294c9937f06b5b16bc4d14ab14cc Mon Sep 17 00:00:00 2001 From: Binh-Minh Date: Wed, 10 Jun 2026 12:39:21 -0400 Subject: [PATCH 3/3] Clean up --- hdf/src/hproto.h | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/hdf/src/hproto.h b/hdf/src/hproto.h index c0d454690..4fbaae53b 100644 --- a/hdf/src/hproto.h +++ b/hdf/src/hproto.h @@ -1408,16 +1408,6 @@ HDFLIBAPI int Vgetclass40(int32 vkey, char *vgclass, size_t *buf_size); /* in 4. HDFLIBAPI int Vinquire40(int32 vkey, int32 *nentries, char *vgname, size_t *buf_size); /* in 4.0 */ -HDFLIBAPI int32 Vgetname(int32 vkey, char *vgname); /* deprecated in 4.0 */ - -HDFLIBAPI int32 Vgetnamelen(int32 vkey, uint16 *name_len); /* deprecated in 4.0 */ - -HDFLIBAPI int32 Vgetclassnamelen(int32 vkey, uint16 *classname_len); /* deprecated in 4.0 */ - -HDFLIBAPI int32 Vgetclass(int32 vkey, char *vgclass); /* deprecated in 4.0 */ - -HDFLIBAPI int Vinquire(int32 vkey, int32 *nentries, char *vgname); /* deprecated in 4.0 */ - HDFLIBAPI int32 Vdelete(int32 f, int32 ref); HDFLIBAPI int Vgisinternal(int32 vkey);