rpc: Fix crash when retrieving attribute length

It is possible that NULL is given to the serializers, when
C_GetAttributeValue() just wants to know the size of an attribute.
Previously, this resulted in giving NULL to memcpy().

diff --git a/p11-kit/rpc-message.c b/p11-kit/rpc-message.c
index 803063f2cd049c6ea7d1c75f84ec696992953634..bccfd2ac1993015445e9ff0ed76c22600187f8df 100644 (file)
--- a/p11-kit/rpc-message.c
+++ b/p11-kit/rpc-message.c
@@ -880,14 +880,15 @@ p11_rpc_buffer_add_byte_value (p11_buffer *buffer,
                               const void *value,
                               CK_ULONG value_length)
 {
-       CK_BYTE byte_value;
+       CK_BYTE byte_value = 0;
 
        /* Check if value can be converted to CK_BYTE. */
        if (value_length > sizeof (CK_BYTE)) {
                p11_buffer_fail (buffer);
                return;
        }
-       memcpy (&byte_value, value, value_length);
+       if (value)
+               memcpy (&byte_value, value, value_length);
 
        /* Check if byte_value can be converted to uint8_t. */
        if (byte_value > UINT8_MAX) {
@@ -903,14 +904,15 @@ p11_rpc_buffer_add_ulong_value (p11_buffer *buffer,
                                const void *value,
                                CK_ULONG value_length)
 {
-       CK_ULONG ulong_value;
+       CK_ULONG ulong_value = 0;
 
        /* Check if value can be converted to CK_ULONG. */
        if (value_length > sizeof (CK_ULONG)) {
                p11_buffer_fail (buffer);
                return;
        }
-       memcpy (&ulong_value, value, value_length);
+       if (value)
+               memcpy (&ulong_value, value, value_length);
 
        /* Check if ulong_value can be converted to uint64_t. */
        if (ulong_value > UINT64_MAX) {

diff --git a/p11-kit/test-rpc.c b/p11-kit/test-rpc.c
index 7c563cf15defd774503571dd638201d74c47dc8c..09f30e0d0476ca366e88e5f21b6649a1373424ff 100644 (file)
--- a/p11-kit/test-rpc.c
+++ b/p11-kit/test-rpc.c
@@ -633,6 +633,33 @@ test_mechanism_value (void)
        p11_rpc_mechanisms_override_supported = mechanisms;
 }
 
+static void
+test_message_write (void)
+{
+       p11_rpc_message msg;
+       p11_buffer buffer;
+       CK_BBOOL truev = CK_TRUE;
+       CK_ULONG zerov = (CK_ULONG)0;
+       char labelv[] = "label";
+       CK_ATTRIBUTE attrs[] = {
+               { CKA_MODIFIABLE, &truev, sizeof (truev) },
+               { CKA_LABEL, labelv, sizeof (labelv) },
+               /* These are cases when C_GetAttributeValue is called
+                * to obtain the length */
+               { CKA_COPYABLE, NULL, sizeof (truev) },
+               { CKA_BITS_PER_PIXEL, NULL, sizeof (zerov) }
+       };
+       bool ret;
+
+       ret = p11_buffer_init (&buffer, 0);
+       assert_num_eq (true, ret);
+       p11_rpc_message_init (&msg, &buffer, &buffer);
+       ret = p11_rpc_message_write_attribute_array (&msg, attrs, ELEMS(attrs));
+       assert_num_eq (true, ret);
+       p11_rpc_message_clear (&msg);
+       p11_buffer_uninit (&buffer);
+}
+
 static p11_virtual base;
 static unsigned int rpc_initialized = 0;
 
@@ -1324,6 +1351,7 @@ main (int argc,
        p11_test (test_date_value, "/rpc/date-value");
        p11_test (test_byte_array_value, "/rpc/byte-array-value");
        p11_test (test_mechanism_value, "/rpc/mechanism-value");
+       p11_test (test_message_write, "/rpc/message-write");
 
        p11_test (test_initialize_fails_on_client, "/rpc/initialize-fails-on-client");
        p11_test (test_initialize_fails_on_server, "/rpc/initialize-fails-on-server");