Skip to content

Commit

Permalink
Issue #1824: Replaced most remaining sprintf with safer snprint (#4003)
Browse files Browse the repository at this point in the history
  • Loading branch information
@seanm
seanm authored Feb 13, 2024
1 parent beee0a3 commit f309fce
Show file tree
Hide file tree
Showing 13 changed files with 93 additions and 90 deletions.
19 changes: 10 additions & 9 deletions java/src/jni/h5util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1192,7 +1192,8 @@ h5str_sprintf(JNIEnv *env, h5str_t *out_str, hid_t container, hid_t tid, void *i
* object.
*/

if (NULL == (this_str = (char *)malloc(64)))
const size_t size = 64;
if (NULL == (this_str = (char *)malloc(size)))
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: failed to allocate string buffer");

if ((obj = H5Rdereference2(container, H5P_DEFAULT, H5R_OBJECT, cptr)) < 0)
Expand All @@ -1206,25 +1207,25 @@ h5str_sprintf(JNIEnv *env, h5str_t *out_str, hid_t container, hid_t tid, void *i

switch (oi.type) {
case H5O_TYPE_GROUP:
if (sprintf(this_str, "%s %s", H5_TOOLS_GROUP, obj_tok_str) < 0)
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: sprintf failure");
if (snprintf(this_str, size, "%s %s", H5_TOOLS_GROUP, obj_tok_str) < 0)
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: snprintf failure");
break;

case H5O_TYPE_DATASET:
if (sprintf(this_str, "%s %s", H5_TOOLS_DATASET, obj_tok_str) < 0)
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: sprintf failure");
if (snprintf(this_str, size, "%s %s", H5_TOOLS_DATASET, obj_tok_str) < 0)
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: snprintf failure");
break;

case H5O_TYPE_NAMED_DATATYPE:
if (sprintf(this_str, "%s %s", H5_TOOLS_DATATYPE, obj_tok_str) < 0)
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: sprintf failure");
if (snprintf(this_str, size, "%s %s", H5_TOOLS_DATATYPE, obj_tok_str) < 0)
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: snprintf failure");
break;

case H5O_TYPE_UNKNOWN:
case H5O_TYPE_NTYPES:
default:
if (sprintf(this_str, "%u-%s", (unsigned)oi.type, obj_tok_str) < 0)
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: sprintf failure");
if (snprintf(this_str, size, "%u-%s", (unsigned)oi.type, obj_tok_str) < 0)
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: snprintf failure");
break;
}

Expand Down
33 changes: 17 additions & 16 deletions src/H5FDfamily.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,27 +234,28 @@ H5FD__family_get_default_printf_filename(const char *old_filename)
HGOTO_ERROR(H5E_VFL, H5E_CANTALLOC, NULL, "can't allocate new filename buffer");

/* Determine if filename contains a ".h5" extension. */
if ((file_extension = strstr(old_filename, ".h5"))) {
file_extension = strstr(old_filename, ".h5");
if (file_extension) {
/* Insert the printf format between the filename and ".h5" extension. */
strcpy(tmp_buffer, old_filename);
file_extension = strstr(tmp_buffer, ".h5");
sprintf(file_extension, "%s%s", suffix, ".h5");
intptr_t beginningLength = file_extension - old_filename;
snprintf(tmp_buffer, new_filename_len, "%.*s%s%s", (int)beginningLength, old_filename, suffix, ".h5");
}
else if ((file_extension = strrchr(old_filename, '.'))) {
char *new_extension_loc = NULL;

else {
/* If the filename doesn't contain a ".h5" extension, but contains
* AN extension, just insert the printf format before that extension.
*/
strcpy(tmp_buffer, old_filename);
new_extension_loc = strrchr(tmp_buffer, '.');
sprintf(new_extension_loc, "%s%s", suffix, file_extension);
}
else {
/* If the filename doesn't contain an extension at all, just insert
* the printf format at the end of the filename.
*/
snprintf(tmp_buffer, new_filename_len, "%s%s", old_filename, suffix);
file_extension = strrchr(old_filename, '.');
if (file_extension) {
intptr_t beginningLength = file_extension - old_filename;
snprintf(tmp_buffer, new_filename_len, "%.*s%s%s", (int)beginningLength, old_filename, suffix,
file_extension);
}
else {
/* If the filename doesn't contain an extension at all, just insert
* the printf format at the end of the filename.
*/
snprintf(tmp_buffer, new_filename_len, "%s%s", old_filename, suffix);
}
}

ret_value = tmp_buffer;
Expand Down
33 changes: 17 additions & 16 deletions src/H5FDsplitter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -532,27 +532,28 @@ H5FD__splitter_get_default_wo_path(char *new_path, size_t new_path_len, const ch
HGOTO_ERROR(H5E_VFL, H5E_CANTSET, FAIL, "filename exceeds max length");

/* Determine if filename contains a ".h5" extension. */
if ((file_extension = strstr(base_filename, ".h5"))) {
file_extension = strstr(base_filename, ".h5");
if (file_extension) {
/* Insert the suffix between the filename and ".h5" extension. */
strcpy(new_path, base_filename);
file_extension = strstr(new_path, ".h5");
sprintf(file_extension, "%s%s", suffix, ".h5");
intptr_t beginningLength = file_extension - base_filename;
snprintf(new_path, new_path_len, "%.*s%s%s", (int)beginningLength, base_filename, suffix, ".h5");
}
else if ((file_extension = strrchr(base_filename, '.'))) {
char *new_extension_loc = NULL;

else {
/* If the filename doesn't contain a ".h5" extension, but contains
* AN extension, just insert the suffix before that extension.
*/
strcpy(new_path, base_filename);
new_extension_loc = strrchr(new_path, '.');
sprintf(new_extension_loc, "%s%s", suffix, file_extension);
}
else {
/* If the filename doesn't contain an extension at all, just insert
* the suffix at the end of the filename.
*/
snprintf(new_path, new_path_len, "%s%s", base_filename, suffix);
file_extension = strrchr(base_filename, '.');
if (file_extension) {
intptr_t beginningLength = file_extension - base_filename;
snprintf(new_path, new_path_len, "%.*s%s%s", (int)beginningLength, base_filename, suffix,
file_extension);
}
else {
/* If the filename doesn't contain an extension at all, just insert
* the suffix at the end of the filename.
*/
snprintf(new_path, new_path_len, "%s%s", base_filename, suffix);
}
}

done:
Expand Down
22 changes: 11 additions & 11 deletions test/API/H5_api_async_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ test_multi_dataset_io(void)
/* Loop over datasets */
for (i = 0; i < 5; i++) {
/* Set dataset name */
sprintf(dset_name, "dset%d", i);
snprintf(dset_name, sizeof(dset_name), "dset%d", i);

/* Create the dataset asynchronously */
if ((dset_id[i] = H5Dcreate_async(file_id, dset_name, H5T_NATIVE_INT, space_id, H5P_DEFAULT,
Expand Down Expand Up @@ -450,7 +450,7 @@ test_multi_dataset_io(void)
/* Loop over datasets */
for (i = 0; i < 5; i++) {
/* Set dataset name */
sprintf(dset_name, "dset%d", i);
snprintf(dset_name, sizeof(dset_name), "dset%d", i);

/* Open the dataset asynchronously */
if ((dset_id[0] = H5Dopen_async(file_id, dset_name, H5P_DEFAULT, es_id)) < 0)
Expand Down Expand Up @@ -479,7 +479,7 @@ test_multi_dataset_io(void)
/* Loop over datasets */
for (i = 0; i < 5; i++) {
/* Set dataset name */
sprintf(dset_name, "dset%d", i);
snprintf(dset_name, sizeof(dset_name), "dset%d", i);

/* Open the dataset asynchronously */
if ((dset_id[0] = H5Dopen_async(file_id, dset_name, H5P_DEFAULT, es_id)) < 0)
Expand Down Expand Up @@ -619,7 +619,7 @@ test_multi_file_dataset_io(void)
/* Loop over files */
for (i = 0; i < 5; i++) {
/* Set file name */
sprintf(file_name, ASYNC_API_TEST_FILE_PRINTF, i);
snprintf(file_name, sizeof(file_name), ASYNC_API_TEST_FILE_PRINTF, i);

/* Create file asynchronously */
if ((file_id[i] =
Expand Down Expand Up @@ -761,7 +761,7 @@ test_multi_file_dataset_io(void)
/* Loop over files */
for (i = 0; i < 5; i++) {
/* Set file name */
sprintf(file_name, ASYNC_API_TEST_FILE_PRINTF, i);
snprintf(file_name, sizeof(file_name), ASYNC_API_TEST_FILE_PRINTF, i);

/* Open the file asynchronously */
if ((file_id[0] = H5Fopen_async(file_name, H5F_ACC_RDWR, H5P_DEFAULT, es_id)) < 0)
Expand Down Expand Up @@ -799,7 +799,7 @@ test_multi_file_dataset_io(void)
/* Loop over files */
for (i = 0; i < 5; i++) {
/* Set file name */
sprintf(file_name, ASYNC_API_TEST_FILE_PRINTF, i);
snprintf(file_name, sizeof(file_name), ASYNC_API_TEST_FILE_PRINTF, i);

/* Open the file asynchronously */
if ((file_id[0] = H5Fopen_async(file_name, H5F_ACC_RDONLY, H5P_DEFAULT, es_id)) < 0)
Expand Down Expand Up @@ -929,7 +929,7 @@ test_multi_file_grp_dset_io(void)
/* Loop over files */
for (i = 0; i < 5; i++) {
/* Set file name */
sprintf(file_name, ASYNC_API_TEST_FILE_PRINTF, i);
snprintf(file_name, sizeof(file_name), ASYNC_API_TEST_FILE_PRINTF, i);

/* Create file asynchronously */
if ((file_id = H5Fcreate_async(file_name, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT, es_id)) <
Expand Down Expand Up @@ -981,7 +981,7 @@ test_multi_file_grp_dset_io(void)
/* Loop over files */
for (i = 0; i < 5; i++) {
/* Set file name */
sprintf(file_name, ASYNC_API_TEST_FILE_PRINTF, i);
snprintf(file_name, sizeof(file_name), ASYNC_API_TEST_FILE_PRINTF, i);

/* Open the file asynchronously */
if ((file_id = H5Fopen_async(file_name, H5F_ACC_RDONLY, H5P_DEFAULT, es_id)) < 0)
Expand Down Expand Up @@ -1039,7 +1039,7 @@ test_multi_file_grp_dset_io(void)
/* Loop over files */
for (i = 0; i < 5; i++) {
/* Set file name */
sprintf(file_name, ASYNC_API_TEST_FILE_PRINTF, i);
snprintf(file_name, sizeof(file_name), ASYNC_API_TEST_FILE_PRINTF, i);

/* Create file asynchronously */
if ((file_id = H5Fcreate_async(file_name, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT, es_id)) <
Expand Down Expand Up @@ -1096,7 +1096,7 @@ test_multi_file_grp_dset_io(void)
/* Loop over files */
for (i = 0; i < 5; i++) {
/* Set file name */
sprintf(file_name, ASYNC_API_TEST_FILE_PRINTF, i);
snprintf(file_name, sizeof(file_name), ASYNC_API_TEST_FILE_PRINTF, i);

/* Open the file asynchronously */
if ((file_id = H5Fopen_async(file_name, H5F_ACC_RDONLY, H5P_DEFAULT, es_id)) < 0)
Expand Down Expand Up @@ -2676,7 +2676,7 @@ cleanup_files(void)

H5Fdelete(ASYNC_API_TEST_FILE, H5P_DEFAULT);
for (i = 0; i <= max_printf_file; i++) {
snprintf(file_name, 64, ASYNC_API_TEST_FILE_PRINTF, i);
snprintf(file_name, sizeof(file_name), ASYNC_API_TEST_FILE_PRINTF, i);
H5Fdelete(file_name, H5P_DEFAULT);
} /* end for */
}
Expand Down
2 changes: 1 addition & 1 deletion test/API/H5_api_attribute_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10725,7 +10725,7 @@ test_attribute_many(void)

/* Create many attributes */
for (u = 0; u < ATTRIBUTE_MANY_NUMB; u++) {
sprintf(attrname, "many-%06u", u);
snprintf(attrname, sizeof(attrname), "many-%06u", u);

if ((attr_id = H5Acreate2(group_id, attrname, attr_dtype, space_id, H5P_DEFAULT, H5P_DEFAULT)) < 0) {
H5_FAILED();
Expand Down
17 changes: 10 additions & 7 deletions test/API/H5_api_dataset_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1218,7 +1218,7 @@ test_create_dataset_random_shapes(void)
goto error;
}

sprintf(name, "%s%zu", DATASET_SHAPE_TEST_DSET_BASE_NAME, i + 1);
snprintf(name, sizeof(name), "%s%zu", DATASET_SHAPE_TEST_DSET_BASE_NAME, i + 1);

if ((dset_id = H5Dcreate2(group_id, name, dset_dtype, space_id, H5P_DEFAULT, H5P_DEFAULT,
H5P_DEFAULT)) < 0) {
Expand Down Expand Up @@ -1316,7 +1316,7 @@ test_create_dataset_predefined_types(void)
generate_random_dataspace(DATASET_PREDEFINED_TYPE_TEST_SPACE_RANK, NULL, NULL, false)) < 0)
TEST_ERROR;

sprintf(name, "%s%zu", DATASET_PREDEFINED_TYPE_TEST_BASE_NAME, i);
snprintf(name, sizeof(name), "%s%zu", DATASET_PREDEFINED_TYPE_TEST_BASE_NAME, i);

if ((dset_id = H5Dcreate2(group_id, name, predefined_type_test_table[i], fspace_id, H5P_DEFAULT,
H5P_DEFAULT, H5P_DEFAULT)) < 0) {
Expand Down Expand Up @@ -2154,7 +2154,8 @@ test_create_dataset_creation_properties(void)
PART_ERROR(DCPL_alloc_time_test);
}

sprintf(name, "%s%zu", DATASET_CREATION_PROPERTIES_TEST_ALLOC_TIMES_BASE_NAME, i);
snprintf(name, sizeof(name), "%s%zu", DATASET_CREATION_PROPERTIES_TEST_ALLOC_TIMES_BASE_NAME,
i);

if ((dset_id = H5Dcreate2(group_id, name, dset_dtype, fspace_id, H5P_DEFAULT, dcpl_id,
H5P_DEFAULT)) < 0) {
Expand Down Expand Up @@ -2230,7 +2231,8 @@ test_create_dataset_creation_properties(void)
PART_ERROR(DCPL_attr_crt_order_test);
}

sprintf(name, "%s%zu", DATASET_CREATION_PROPERTIES_TEST_CRT_ORDER_BASE_NAME, i);
snprintf(name, sizeof(name), "%s%zu", DATASET_CREATION_PROPERTIES_TEST_CRT_ORDER_BASE_NAME,
i);

if ((dset_id = H5Dcreate2(group_id, name, dset_dtype, fspace_id, H5P_DEFAULT, dcpl_id,
H5P_DEFAULT)) < 0) {
Expand Down Expand Up @@ -2363,7 +2365,8 @@ test_create_dataset_creation_properties(void)
PART_ERROR(DCPL_fill_time_property_test);
}

sprintf(name, "%s%zu", DATASET_CREATION_PROPERTIES_TEST_FILL_TIMES_BASE_NAME, i);
snprintf(name, sizeof(name), "%s%zu", DATASET_CREATION_PROPERTIES_TEST_FILL_TIMES_BASE_NAME,
i);

if ((dset_id = H5Dcreate2(group_id, name, dset_dtype, fspace_id, H5P_DEFAULT, dcpl_id,
H5P_DEFAULT)) < 0) {
Expand Down Expand Up @@ -2931,7 +2934,7 @@ test_create_dataset_creation_properties(void)
}
}

sprintf(name, "%s%zu", DATASET_CREATION_PROPERTIES_TEST_LAYOUTS_BASE_NAME, i);
snprintf(name, sizeof(name), "%s%zu", DATASET_CREATION_PROPERTIES_TEST_LAYOUTS_BASE_NAME, i);

if ((dset_id =
H5Dcreate2(group_id, name, (H5D_COMPACT == layouts[i]) ? compact_dtype : dset_dtype,
Expand Down Expand Up @@ -3192,7 +3195,7 @@ test_create_many_dataset(void)
printf("\n");
for (i = 0; i < DATASET_NUMB; i++) {
printf("\r %u/%u", i + 1, DATASET_NUMB);
sprintf(dset_name, "dset_%02u", i);
snprintf(dset_name, sizeof(dset_name), "dset_%02u", i);
data = i % 256;

if ((dset_id = H5Dcreate2(group_id, dset_name, H5T_NATIVE_UCHAR, dataspace_id, H5P_DEFAULT,
Expand Down
2 changes: 1 addition & 1 deletion test/API/H5_api_file_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -948,7 +948,7 @@ test_flush_file(void)
}

for (u = 0; u < 10; u++) {
sprintf(dset_name, "Dataset %u", u);
snprintf(dset_name, sizeof(dset_name), "Dataset %u", u);

if ((dset_id = H5Dcreate2(file_id, dset_name, H5T_STD_U32LE, dspace_id, H5P_DEFAULT, H5P_DEFAULT,
H5P_DEFAULT)) < 0) {
Expand Down
8 changes: 4 additions & 4 deletions test/API/H5_api_group_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ test_create_many_groups(void)
printf("\n");
for (i = 0; i < GROUP_NUMB_MANY; i++) {
printf("\r %u/%u", i + 1, GROUP_NUMB_MANY);
sprintf(group_name, "group %02u", i);
snprintf(group_name, sizeof(group_name), "group %02u", i);
if ((child_group_id =
H5Gcreate2(parent_group_id, group_name, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0) {
H5_FAILED();
Expand Down Expand Up @@ -342,11 +342,11 @@ create_group_recursive(hid_t parent_gid, unsigned counter)

printf("\r %u/%u", counter, GROUP_DEPTH);
if (counter == 1)
sprintf(gname, "2nd_child_group");
snprintf(gname, sizeof(gname), "2nd_child_group");
else if (counter == 2)
sprintf(gname, "3rd_child_group");
snprintf(gname, sizeof(gname), "3rd_child_group");
else
sprintf(gname, "%dth_child_group", counter + 1);
snprintf(gname, sizeof(gname), "%dth_child_group", counter + 1);
if ((child_gid = H5Gcreate2(parent_gid, gname, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0) {
H5_FAILED();
printf(" couldn't create group '%s'\n", gname);
Expand Down
10 changes: 2 additions & 8 deletions test/filter_plugin.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -847,10 +847,7 @@ test_creating_groups_using_plugins(hid_t fid)

/* Create multiple groups under the top-level group */
for (i = 0; i < N_SUBGROUPS; i++) {
char *sp = subgroup_name;

sp += snprintf(subgroup_name, sizeof(subgroup_name), SUBGROUP_PREFIX);
sprintf(sp, "%d", i);
snprintf(subgroup_name, sizeof(subgroup_name), SUBGROUP_PREFIX "%d", i);

if ((sub_gid = H5Gcreate2(gid, subgroup_name, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0)
TEST_ERROR;
Expand Down Expand Up @@ -906,10 +903,7 @@ test_opening_groups_using_plugins(hid_t fid)

/* Open all the sub-groups under the top-level group */
for (i = 0; i < N_SUBGROUPS; i++) {
char *sp = subgroup_name;

sp += snprintf(subgroup_name, sizeof(subgroup_name), SUBGROUP_PREFIX);
sprintf(sp, "%d", i);
snprintf(subgroup_name, sizeof(subgroup_name), SUBGROUP_PREFIX "%d", i);

if ((sub_gid = H5Gopen2(gid, subgroup_name, H5P_DEFAULT)) < 0)
TEST_ERROR;
Expand Down
Loading

0 comments on commit f309fce

Please sign in to comment.