From 57926d549ef75c532badc7ad3318170f7130295b Mon Sep 17 00:00:00 2001
From: Dana Robinson <43805+derobins@users.noreply.github.com>
Date: Mon, 8 Aug 2022 11:00:13 -0700
Subject: Fixes some badness in the onion VFD revision comment code (#1975)

* Fixes some badness in the revision comment code

* Committing clang-format changes

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
---
 src/H5FDonion.c | 25 ++++++++++++++++---------
 src/H5FDonion.h |  8 ++++----
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/src/H5FDonion.c b/src/H5FDonion.c
index 23ea624..6cdd9a4 100644
--- a/src/H5FDonion.c
+++ b/src/H5FDonion.c
@@ -1182,16 +1182,23 @@ H5FD__onion_open(const char *filename, unsigned flags, hid_t fapl_id, haddr_t ma
 
     /* Copy comment from FAPL info, if one is given */
     if ((H5F_ACC_RDWR | H5F_ACC_CREAT | H5F_ACC_TRUNC) & flags) {
-        if (fa->comment) {
-            /* Free the old comment */
-            file->curr_rev_record.comment = H5MM_xfree(file->curr_rev_record.comment);
-
-            /* TODO: Lengths of strings should be size_t */
-            file->curr_rev_record.comment_size = (uint32_t)HDstrlen(fa->comment) + 1;
+        /* Free the old comment */
+        file->curr_rev_record.comment = H5MM_xfree(file->curr_rev_record.comment);
+
+        /* The buffer is of size H5FD_ONION_FAPL_INFO_COMMENT_MAX_LEN + 1
+         *
+         * We're getting this buffer from a fixed-size array in a struct, which
+         * will be garbage and not null-terminated if the user isn't careful.
+         * Be careful of this and do strndup first to ensure strdup gets a
+         * null-termianted string (HDF5 doesn't provide a strnlen call if you
+         * don't have one).
+         */
+        if (NULL ==
+            (file->curr_rev_record.comment = H5MM_strndup(fa->comment, H5FD_ONION_FAPL_INFO_COMMENT_MAX_LEN)))
+            HGOTO_ERROR(H5E_RESOURCE, H5E_CANTALLOC, NULL, "unable to duplicate comment string")
 
-            if (NULL == (file->curr_rev_record.comment = H5MM_xstrdup(fa->comment)))
-                HGOTO_ERROR(H5E_RESOURCE, H5E_CANTALLOC, NULL, "unable to allocate comment string")
-        }
+        /* TODO: Lengths of strings should be size_t */
+        file->curr_rev_record.comment_size = (uint32_t)HDstrlen(fa->comment) + 1;
     }
     file->origin_eof  = file->header.origin_eof;
     file->logical_eof = MAX(file->curr_rev_record.logical_eof, file->logical_eof);
diff --git a/src/H5FDonion.h b/src/H5FDonion.h
index 04fd2ff..cc767c6 100644
--- a/src/H5FDonion.h
+++ b/src/H5FDonion.h
@@ -33,12 +33,12 @@
 /* Flag to require page alignment of onion revision data */
 #define H5FD_ONION_FAPL_INFO_CREATE_FLAG_ENABLE_PAGE_ALIGNMENT 0x2
 
-/* Max length of a comment */
+/* Max length of a comment
+ * The buffer is defined to be this size + 1 to handle the NUL
+ */
 #define H5FD_ONION_FAPL_INFO_COMMENT_MAX_LEN 255
 
-/* Indicates that you want the latest revision
- * TODO: Does this work?
- */
+/* Indicates that you want the latest revision */
 #define H5FD_ONION_FAPL_INFO_REVISION_ID_LATEST UINT64_MAX
 
 typedef enum H5FD_onion_target_file_constant_t {
-- 
cgit v0.12