Clean up format_hex_ex()

Cherry-pick of commit 29404010 from master, slightly reworked to match the
2.3 codebase (no flags inside space_break here), and pulled in the new
static_assert() fallback we also have in master now.

Fix a potential null-pointer dereference, and make the code a bit more
readable while doing so.

The NULL dereference could not be triggered, because the current code
never called format_hex_ex() with maxouput == 0 and separator == NULL.
But it's nicer to not depend on that.

Our use of int vs size_t for lengths needs some attention too, but I'm
not pulling that into this patch.  Instead I decided to just make the
(previously existing) assumption that INT_MAX <= SIZE_MAX explicit by
adding a static_assert().

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1480352107-19652-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13269.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index fb3b52d1747fff1a485693d67524fcd5ebf055c4..edbdd666f82f9759c39ddc63e945c3dbc8e054af 100644 (file)
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -400,9 +400,13 @@ format_hex_ex (const uint8_t *data, int size, int maxoutput,
               int space_break, const char* separator,
               struct gc_arena *gc)
 {
-  struct buffer out = alloc_buf_gc (maxoutput ? maxoutput :
-                                   ((size * 2) + (size / space_break) * (int) strlen (separator) + 2),
-                                   gc);
+  const size_t separator_len = separator ? strlen (separator) : 0;
+  static_assert (INT_MAX <= SIZE_MAX, "Code assumes INT_MAX <= SIZE_MAX");
+  const size_t out_len = maxoutput > 0 ? maxoutput :
+           ((size * 2) + ((size / space_break) * separator_len) + 2);
+
+  struct buffer out = alloc_buf_gc (out_len, gc);
+
   int i;
   for (i = 0; i < size; ++i)
     {

diff --git a/src/openvpn/error.h b/src/openvpn/error.h
index 36819e0a8bc40d5495798831e14037722d819186..4d7fef96e87e43ef747387a42ad1cd5c975ab786 100644 (file)
--- a/src/openvpn/error.h
+++ b/src/openvpn/error.h
@@ -217,6 +217,14 @@ FILE *msg_fp(const unsigned int flags);
 void assert_failed (const char *filename, int line, const char *condition)
   __attribute__((__noreturn__));
 
+/* Poor-man's static_assert() for when not supplied by assert.h, taken from
+ * Linux's sys/cdefs.h under GPLv2 */
+#ifndef static_assert
+#define static_assert(expr, diagnostic) \
+    extern int (*__OpenVPN_static_assert_function (void)) \
+      [!!sizeof (struct { int __error_if_negative: (expr) ? 2 : -1; })]
+#endif
+
 #ifdef ENABLE_DEBUG
 void crash (void); /* force a segfault (debugging only) */
 #endif