utils: Handle NULL consistently if memwipe() is implemented via explicit_bzero()

Our own implementation ignores NULL values, however, explicit_bzero()
can't handle that, as indicated by the `__nonnull ((1))` attribute in the
function's signature in string.h, and causes a segmentation fault.  This
was noticed in one of the unit tests for NewHope.  Since we usually use
memwipe() via chunk_clear(), which already ignores NULL pointers, this
is not that much of an issue in practice.

Fixes: 149d1bbb055a ("memory: Use explicit_bzero() as memwipe() if available")

diff --git a/src/libstrongswan/tests/suites/test_utils.c b/src/libstrongswan/tests/suites/test_utils.c
index 27343349e5ab927b36b37d64e9716111f2222d5a..9060f57b589a281ca259af4f567c02e4c21dd434 100644 (file)
--- a/src/libstrongswan/tests/suites/test_utils.c
+++ b/src/libstrongswan/tests/suites/test_utils.c
@@ -547,6 +547,63 @@ START_TEST(test_memstr)
 }
 END_TEST
 
+/*******************************************************************************
+ * memwipe
+ */
+
+START_TEST(test_memwipe_null)
+{
+       memwipe(NULL, 16);
+}
+END_TEST
+
+static inline bool test_wiped_memory(char *buf, size_t len)
+{
+       int i;
+
+       /* comparing two bytes at once reduces the chances of conflicts if memory
+        * got overwritten already */
+       for (i = 0; i < len; i += 2)
+       {
+               if (buf[i] != 0 && buf[i] == i &&
+                       buf[i+1] != 0 && buf[i+1] == i+1)
+               {
+                       return FALSE;
+               }
+       }
+       return TRUE;
+}
+
+START_TEST(test_memwipe_stack)
+{
+       char buf[64];
+       int i;
+
+       for (i = 0; i < sizeof(buf); i++)
+       {
+               buf[i] = i;
+       }
+       memwipe(buf, sizeof(buf));
+       ck_assert(test_wiped_memory(buf, sizeof(buf)));
+}
+END_TEST
+
+START_TEST(test_memwipe_heap)
+{
+       size_t len = 64;
+       char *buf = malloc(len);
+       int i;
+
+       for (i = 0; i < len; i++)
+       {
+               buf[i] = i;
+       }
+       memwipe(buf, len);
+       ck_assert(test_wiped_memory(buf, len));
+       free(buf);
+}
+END_TEST
+
 /*******************************************************************************
  * utils_memrchr
  */
@@ -1132,6 +1189,12 @@ Suite *utils_suite_create()
        tcase_add_loop_test(tc, test_memstr, 0, countof(memstr_data));
        suite_add_tcase(s, tc);
 
+       tc = tcase_create("memwipe");
+       tcase_add_test(tc, test_memwipe_null);
+       tcase_add_test(tc, test_memwipe_stack);
+       tcase_add_test(tc, test_memwipe_heap);
+       suite_add_tcase(s, tc);
+
        tc = tcase_create("utils_memrchr");
        tcase_add_loop_test(tc, test_utils_memrchr, 0, countof(memrchr_data));
        suite_add_tcase(s, tc);

diff --git a/src/libstrongswan/utils/utils/memory.h b/src/libstrongswan/utils/utils/memory.h
index c257b1eca7eb61f50c34807e4a9fd355bd476e10..eea19588ef04829b50b6040ef9d832b82aecdc86 100644 (file)
--- a/src/libstrongswan/utils/utils/memory.h
+++ b/src/libstrongswan/utils/utils/memory.h
@@ -87,7 +87,13 @@ static inline void *memset_noop(void *s, int c, size_t n)
 void memxor(uint8_t dest[], const uint8_t src[], size_t n);
 
 #ifdef HAVE_EXPLICIT_BZERO
-#define memwipe(ptr, n) explicit_bzero(ptr, n)
+static inline void memwipe(void *ptr, size_t n)
+{
+       if (ptr)
+       {
+               explicit_bzero(ptr, n);
+       }
+}
 #else /* HAVE_EXPLICIT_BZERO */
 /**
  * Safely overwrite n bytes of memory at ptr with zero, non-inlining variant.