out of bounds write in H5FD_s3comms_aws_canonical_request

[mwhudson]

hdf5 was failing to build in ubuntu on s390x, seen here https://launchpad.net/ubuntu/+source/hdf5/1.10.7+repack-4/+build/22296161 and I feared some kind of endianness thing but instead it was a failure in test_aws_canonical_request that turned out to be caused by an out of bounds write in H5FD_s3comms_aws_canonical_request, specifically

    /* remove trailing ';' from signed headers sequence */
    signed_headers_dest[HDstrlen(signed_headers_dest) - 1] = '\0';

when signed_headers_dest is the empty string, which happens for the last of the test cases in the function (this manifested as a scribble over a different local variable's value in test_aws_canonical_request's stack frame so took a bit of figuring out).

I've no idea why this only just started failing for us or why it only fails on s390x but either way it's clearly buggy code.

[mwhudson]

On reflection I guess this bug does have a degree of endian dependence: on a little endian system (with the same stack layout) it would result in a NUL being written to the most significant byte of a heap pointer, and that's not going to do anything.

Anyway, we applied the following very unsophisticated patch in Ubuntu:

--- a/src/H5FDs3comms.c
+++ b/src/H5FDs3comms.c
@@ -1717,8 +1717,10 @@
         node = node->next;
     } /* end while node is not NULL */
 
-    /* remove trailing ';' from signed headers sequence */
-    signed_headers_dest[HDstrlen(signed_headers_dest) - 1] = '\0';
+    if (*signed_headers_dest != '\0') {
+        /* remove trailing ';' from signed headers sequence */
+        signed_headers_dest[HDstrlen(signed_headers_dest) - 1] = '\0';
+    }
 
     /* append signed headers and payload hash
      * NOTE: at present, no HTTP body is handled, per the nature of

[derobins]

Fixed w/ #3681