safer use of memcpy() and memchr(): we must not pass in NULL pointers

* The C standard actually forbids this (undefined behaviour) even though
  it seems intuitive that something like `memcpy(foo, NULL, 0)` does no harm.
* It turned out, there were actual real bugs related to this.
  If memchr() was called with a variable that can be NULL,
  the compiler could assume that the variable is actually always non-NULL
  (since glibc declares memchr() with nonnull), consequently eliminating
  checks for NULL afterwards.
  The same could theoretically happen with memcpy().
  This manifested itself in the empty search crashing when building with -O3.
  Test case:
  sciteco -e '@S//'
* Consequently, the nightly builds (at least for Ubuntu) also had this bug.
* In some cases, the passed in pointers are passed down from the caller but
  should not be NULL, so I added runtime assertions to guard against it.

6 files changed, 23 insertions, 6 deletions

diff --git a/src/cmdline.c b/src/cmdline.c
index 5c080af..199bc9a 100644
--- a/src/cmdline.c
+++ b/src/cmdline.c
@@ -279,6 +279,8 @@ teco_cmdline_keypress_c(gchar key, GError **error)
 gboolean
 teco_cmdline_fnmacro(const gchar *name, GError **error)
 {
+	g_assert(name != NULL);
+
 	/*
 	 * NOTE: It should be safe to allocate on the stack since
 	 * there are only a limited number of possible function key macros.
diff --git a/src/goto-commands.c b/src/goto-commands.c
index 792b4e3..eb4674b 100644
--- a/src/goto-commands.c
+++ b/src/goto-commands.c
@@ -109,7 +109,7 @@ teco_state_goto_done(teco_machine_main_t *ctx, const teco_string_t *str, GError
 	teco_string_t label = {NULL, 0};
 	while (value > 0) {
 		label.data = label.data ? label.data+label.len+1 : str->data;
-		const gchar *p = memchr(label.data, ',', str->len - (label.data - str->data));
+		const gchar *p = label.data ? memchr(label.data, ',', str->len - (label.data - str->data)) : NULL;
 		label.len = p ? p - label.data : str->len - (label.data - str->data);
 
 		value--;
diff --git a/src/goto.c b/src/goto.c
index 38717f3..fd74442 100644
--- a/src/goto.c
+++ b/src/goto.c
@@ -165,7 +165,8 @@ teco_goto_table_undo_set(teco_goto_table_t *ctx, const gchar *name, gsize len, g
 		token->table = ctx;
 		token->pc = pc;
 		token->len = len;
-		memcpy(token->name, name, len);
+		if (name)
+			memcpy(token->name, name, len);
 	}
 }
 
diff --git a/src/interface-curses/interface.c b/src/interface-curses/interface.c
index ce9207e..4351379 100644
--- a/src/interface-curses/interface.c
+++ b/src/interface-curses/interface.c
@@ -1187,6 +1187,7 @@ teco_interface_get_clipboard(const gchar *name, gchar **str, gsize *len, GError
 	 * (PDCurses does not guarantee that either).
 	 */
 	if (str) {
+		g_assert(contents != NULL);
 		*str = memcpy(g_malloc(length + 1), contents, length);
 		(*str)[length] = '\0';
 	}
diff --git a/src/string-utils.h b/src/string-utils.h
index 40f1b21..3fb37dc 100644
--- a/src/string-utils.h
+++ b/src/string-utils.h
@@ -67,7 +67,8 @@ static inline void
 teco_string_init(teco_string_t *target, const gchar *str, gsize len)
 {
 	target->data = g_malloc(len + 1);
-	memcpy(target->data, str, len);
+	if (str)
+		memcpy(target->data, str, len);
 	target->len = len;
 	target->data[target->len] = '\0';
 }
@@ -98,7 +99,8 @@ static inline void
 teco_string_append(teco_string_t *target, const gchar *str, gsize len)
 {
 	target->data = g_realloc(target->data, target->len + len + 1);
-	memcpy(target->data + target->len, str, len);
+	if (str)
+		memcpy(target->data + target->len, str, len);
 	target->len += len;
 	target->data[target->len] = '\0';
 }
@@ -147,10 +149,17 @@ gint teco_string_casecmp(const teco_string_t *a, const gchar *b, gsize b_len);
 static inline gboolean
 teco_string_contains(const teco_string_t *str, gchar chr)
 {
-	return memchr(str->data, chr, str->len) != NULL;
+	return str->data && memchr(str->data, chr, str->len);
 }
 
-/** @memberof teco_string_t */
+/**
+ * Get index of character in string.
+ *
+ * @return Index of character in string. 0 refers to the first character.
+ *         In case of search failure, a negative value is returned.
+ *
+ * @memberof teco_string_t
+ */
 static inline gint
 teco_string_rindex(const teco_string_t *str, gchar chr)
 {
diff --git a/tests/testsuite.at b/tests/testsuite.at
index e087593..c476ec8 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -93,6 +93,10 @@ AT_SETUP([Q-Register stack cleanup])
 AT_CHECK([$SCITECO -e '@<:@a'], 0, ignore, ignore)
 AT_CLEANUP
 
+AT_SETUP([Empty search])
+AT_CHECK([$SCITECO -e '@S//'], 0, ignore, ignore)
+AT_CLEANUP
+
 AT_BANNER([Known Bugs])
 
 AT_SETUP([Pattern matching overflow])