Rewrite librecrypt_add_algorithm to use string building functions

Signed-off-by: Mattias Andrée <m@maandree.se>

7 files changed, 143 insertions, 94 deletions

diff --git a/Makefile b/Makefile
index 55918d4..364d44c 100644
--- a/Makefile
+++ b/Makefile
@@ -70,6 +70,7 @@ OBJ_PRIVATE =\
 	librecrypt_get_pepper_.o\
 	librecrypt_concat_void_.o\
 	librecrypt_concat_mem_.o\
+	librecrypt_concat_memmove_.o\
 	librecrypt_concat_str_.o\
 	librecrypt_concat_uint_.o\
 	librecrypt_concat_char_no_nul_.o\
diff --git a/common.h b/common.h
index 48608e4..9f4de41 100644
--- a/common.h
+++ b/common.h
@@ -427,13 +427,16 @@ librecrypt_concat_void_(struct concat_state *state, size_t len)
  * does not overrun it's buffer but is still NUL
  * terminated (assuming the buffer is not zero-sized)
  *
+ * If `text` may alias `state->buf`, use
+ * `librecrypt_concat_memmove_` instead
+ *
  * @param  state  The buffer to write to, see `struct concat_state` for more information
  * @param  text   The text to copy to the buffer
  * @param  len    The length of `text`, in bytes
  */
 LIBRECRYPT_NONNULL_1__ LIBRECRYPT_READ_MEM__(2, 3) HIDDEN
 inline void
-librecrypt_concat_mem_(struct concat_state *state, const char *text, size_t len)
+librecrypt_concat_mem_(struct concat_state *state, const char *restrict text, size_t len)
 {
 	char *buf = state->buf;
 	size_t n = librecrypt_concat_void_(state, len);
@@ -449,12 +452,44 @@ librecrypt_concat_mem_(struct concat_state *state, const char *text, size_t len)
  * does not overrun it's buffer but is still NUL
  * terminated (assuming the buffer is not zero-sized)
  *
+ * If `text` cannot alias `state->buf`, use
+ * `librecrypt_concat_mem_` instead
+ *
+ * @param  state  The buffer to write to, see `struct concat_state` for more information
+ * @param  text   The text to copy to the buffer
+ * @param  len    The length of `text`, in bytes
+ */
+LIBRECRYPT_NONNULL_1__ LIBRECRYPT_READ_MEM__(2, 3) HIDDEN
+inline void
+librecrypt_concat_memmove_(struct concat_state *state, const char *text, size_t len)
+{
+	char *buf = state->buf;
+	size_t n = state->size ? MIN(state->size - 1u, len) : 0u;
+	if (buf != text && n)
+		memmove(buf, text, n);
+	state->buf = &state->buf[n];
+	state->size -= n;
+	state->len += len;
+	if (state->size)
+		state->buf[0u] = '\0';
+}
+
+
+/**
+ * Concatenate two strings
+ *
+ * Truncation is done to ensure the resulting string
+ * does not overrun it's buffer but is still NUL
+ * terminated (assuming the buffer is not zero-sized)
+ *
+ * If `text` cannot alias `state->buf`
+ *
  * @param  state  The buffer to write to, see `struct concat_state` for more information
  * @param  text   The text to copy to the buffer
  */
 LIBRECRYPT_NONNULL__ LIBRECRYPT_READ_STR__(2) HIDDEN
 inline void
-librecrypt_concat_str_(struct concat_state *state, const char *text)
+librecrypt_concat_str_(struct concat_state *state, const char *restrict text)
 {
 	librecrypt_concat_mem_(state, text, strlen(text));
 }
diff --git a/librecrypt_add_algorithm.c b/librecrypt_add_algorithm.c
index 4556c9b..b9fa71a 100644
--- a/librecrypt_add_algorithm.c
+++ b/librecrypt_add_algorithm.c
@@ -4,45 +4,26 @@
 
 
 static size_t
-truncated_asterisk(char *out_buffer, size_t out_buffer_size, size_t asterisk)
-{
-	struct concat_state concat_state = {out_buffer, out_buffer_size, 0u};
-	librecrypt_concat_str_(&concat_state, "*");
-	librecrypt_concat_uint_(&concat_state, asterisk);
-	if (concat_state.len < 2u)
-		abort(); /* $covered$ (impossible) */
-	return concat_state.len;
-}
-
-
-static size_t
 measure_asterisk(size_t asterisk)
 {
-	return truncated_asterisk(NULL, 0u, asterisk);
+	struct concat_state concat_state = {NULL, 0u, 0u};
+	librecrypt_concat_uint_(&concat_state, asterisk);
+	return (sizeof("*") - 1u) + concat_state.len;
 }
 
 
 ssize_t
-librecrypt_add_algorithm(char *out_buffer, size_t size, const char *augend,
+librecrypt_add_algorithm(char *out_buffer0, size_t size0, const char *augend,
                          const char *restrict augment, LIBRECRYPT_CONTEXT *ctx)
 {
-	/* TODO rewrite to use the string building functions */
-
-	size_t prefix1, prefix2, min, ret, len, phraselen;
+	struct concat_state concat_state = {out_buffer0, size0, 0u};
+	size_t prefix1, prefix2, len, conf_len, phraselen;
 	size_t hashsize1, hashsize2, asterisk_len;
 	char *phrase, pad;
-	int strict_pad, nul_term, saved_errno;
+	int strict_pad, saved_errno;
 	const unsigned char *lut;
 	ssize_t r;
 
-	/* Reserve space for NUL-termination */
-	if (size) {
-		nul_term = 1;
-		size -= 1u;
-	} else {
-		nul_term = 0;
-	}
-
 	/* Get the prefix and hash size in `augend` and `augment` */
 	prefix1 = librecrypt_settings_prefix(augend, &hashsize1, ctx);
 	prefix2 = librecrypt_settings_prefix(augment, &hashsize2, ctx);
@@ -60,54 +41,31 @@ librecrypt_add_algorithm(char *out_buffer, size_t size, const char *augend,
 			prefix2 += strlen(&augment[prefix2]);
 			hashsize2 = 0u;
 		}
-		ret = prefix1 + 1u + prefix2;
-		if (size) {
-			min = MIN(prefix1, size);
-			if (out_buffer != augend)
-				memmove(out_buffer, augend, min);
-			out_buffer = &out_buffer[min];
-			size -= min;
-			if (size) {
-				*out_buffer++ = LIBRECRYPT_ALGORITHM_LINK_DELIMITER;
-				size -= 1u;
-			}
-			min = MIN(prefix2, size);
-			memcpy(out_buffer, augment, min);
-			out_buffer = &out_buffer[min];
-			size -= min;
-			if (hashsize2) {
-				asterisk_len = truncated_asterisk(out_buffer, size + 1u, hashsize2);
-				if (ret > SIZE_MAX - asterisk_len)
-					abort(); /* $covered$ (impossible) */
-				ret += asterisk_len;
-			} else {
-				out_buffer[0u] = '\0';
-			}
-		} else {
-#if defined(__GNUC__)
-# pragma GCC diagnostic push
-# pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
-#endif
-			if (!hashsize2)
-				goto out;
-#if defined(__GNUC__)
-# pragma GCC diagnostic pop
-#endif
-			asterisk_len = measure_asterisk(hashsize2);
-			if (ret > SIZE_MAX - asterisk_len)
+		switch (concat_state.size) {
+		default:
+			librecrypt_concat_memmove_(&concat_state, augend, prefix1);
+			librecrypt_concat_char_no_nul_(&concat_state, LIBRECRYPT_ALGORITHM_LINK_DELIMITER);
+			librecrypt_concat_mem_(&concat_state, augment, prefix2);
+			break;
+		case 1u:
+			concat_state.buf[0u] = '\0';
+			/* fall through */
+		case 0u:
+			if (librecrypt_post_concat_adjust_(&concat_state, prefix1 + 1u + prefix2))
 				abort(); /* $covered$ (impossible) */
-			ret += asterisk_len;
-		out:
-			if (nul_term)
-				out_buffer[0u] = '\0';
+			break;
 		}
-		if (ret > (size_t)SSIZE_MAX) {
+		if (hashsize2) {
+			librecrypt_concat_char_no_nul_(&concat_state, '*');
+			librecrypt_concat_uint_(&concat_state, hashsize2);
+		}
+		if (concat_state.len > (size_t)SSIZE_MAX) {
 			/* $covered{$ (manually) */
 			errno = EOVERFLOW;
 			return -1;
 			/* $covered}$ */
 		}
-		return (ssize_t)ret;
+		return (ssize_t)concat_state.len;
 	}
 
 	/* Measure size of hash size specification for `augend` */
@@ -117,13 +75,16 @@ librecrypt_add_algorithm(char *out_buffer, size_t size, const char *augend,
 		asterisk_len = 0;
 	}
 
-	/* Measure `augend` and '>' in output */
+	/* Measure `augend`, '>', and `augment` in output */
 	if (prefix1 > SIZE_MAX - 1u - asterisk_len)
 		abort(); /* $covered$ (impossible) */
-	ret = prefix1 + asterisk_len + 1u;
+	conf_len = prefix1 + asterisk_len + 1u;
+	if (prefix2 > SIZE_MAX - conf_len)
+		abort(); /* $covered$ (impossible) */
+	conf_len += prefix2;
 
 	/* Decode the hash from base-64 to binary */
-	if (size <= ret + prefix2) {
+	if (concat_state.size <= conf_len) {
 		/* If the new hash doesn't fit, don't bother;
 		 * hash sizes are independent of password size */
 		phrase = NULL;
@@ -155,27 +116,17 @@ librecrypt_add_algorithm(char *out_buffer, size_t size, const char *augend,
 	}
 
 	/* Chain the hash algorithms: write `augend` */
-	min = MIN(prefix1, size);
-	if (out_buffer != augend && min)
-		memmove(out_buffer, augend, min);
-	out_buffer = &out_buffer[min];
-	size -= min;
-	if (hashsize1 && size) {
-		if (truncated_asterisk(out_buffer, size + 1u, hashsize1) != asterisk_len)
-			abort(); /* $covered$ (impossible reliably) */
-		min = MIN(asterisk_len, size);
-		out_buffer = &out_buffer[min];
-		size -= min;
+	librecrypt_concat_memmove_(&concat_state, augend, prefix1);
+	if (hashsize1) {
+		librecrypt_concat_char_no_nul_(&concat_state, '*');
+		librecrypt_concat_uint_(&concat_state, hashsize1);
 	}
 
 	/* Chain the hash algorithms: write '>' */
-	if (size) {
-		*out_buffer++ = LIBRECRYPT_ALGORITHM_LINK_DELIMITER;
-		size -= 1u;
-	}
+	librecrypt_concat_char_no_nul_(&concat_state, LIBRECRYPT_ALGORITHM_LINK_DELIMITER);
 
 	/* Chain the hash algorithms: write `augment` and hash */
-	r = librecrypt_crypt(out_buffer, nul_term ? size + 1u : 0u, phrase, phraselen, augment, ctx);
+	r = librecrypt_crypt(concat_state.buf, concat_state.size, phrase, phraselen, augment, ctx);
 	if (r <= 0) {
 		saved_errno = errno;
 		librecrypt_wipe(phrase, phraselen);
@@ -185,7 +136,7 @@ librecrypt_add_algorithm(char *out_buffer, size_t size, const char *augend,
 		errno = saved_errno;
 		return -1;
 	}
-	if (ret > (size_t)(SSIZE_MAX - r)) {
+	if (concat_state.len > (size_t)(SSIZE_MAX - r)) {
 		/* $covered{$ (manually) */
 		librecrypt_wipe(phrase, phraselen);
 		free(phrase);
@@ -193,11 +144,11 @@ librecrypt_add_algorithm(char *out_buffer, size_t size, const char *augend,
 		return -1;
 		/* $covered}$ */
 	}
-	ret += (size_t)r;
+	concat_state.len += (size_t)r;
 
 	librecrypt_wipe(phrase, phraselen);
 	free(phrase);
-	return (ssize_t)ret;
+	return (ssize_t)concat_state.len;
 }
 
 
diff --git a/librecrypt_concat_mem_.c b/librecrypt_concat_mem_.c
index 296303e..ab85953 100644
--- a/librecrypt_concat_mem_.c
+++ b/librecrypt_concat_mem_.c
@@ -3,7 +3,7 @@
 #ifndef TEST
 
 
-extern inline void librecrypt_concat_mem_(struct concat_state *state, const char *text, size_t len);
+extern inline void librecrypt_concat_mem_(struct concat_state *state, const char *restrict text, size_t len);
 
 
 #else
diff --git a/librecrypt_concat_memmove_.c b/librecrypt_concat_memmove_.c
new file mode 100644
index 0000000..d25ca44
--- /dev/null
+++ b/librecrypt_concat_memmove_.c
@@ -0,0 +1,62 @@
+/* See LICENSE file for copyright and license details. */
+#include "common.h"
+#ifndef TEST
+
+
+extern inline void librecrypt_concat_memmove_(struct concat_state *state, const char *text, size_t len);
+
+
+#else
+
+
+int
+main(void)
+{
+	char buf[5];
+	struct concat_state state = {buf, sizeof(buf), 0u};
+
+	SET_UP_ALARM();
+	INIT_RESOURCE_TEST();
+
+	memset(buf, 'x', sizeof(buf));
+
+	librecrypt_concat_memmove_(&state, "abcdef", 6u);
+	assert(!memcmp(buf, "abcd\0", sizeof(buf)));
+	assert(state.buf == &buf[4u]);
+	assert(state.size == 1u);
+	assert(state.len == 6u);
+	librecrypt_concat_memmove_(&state, "z", 1u);
+	assert(!memcmp(buf, "abcd\0", sizeof(buf)));
+	assert(state.len == 7u);
+
+	state = (struct concat_state){NULL, 0u, 0u};
+	librecrypt_concat_memmove_(&state, "abc", 3u);
+	assert(state.len == 3u);
+
+	buf[0u] = 'a';
+	buf[1u] = 'b';
+	buf[2u] = 'c';
+	buf[3u] = 'd';
+	buf[4u] = 'e';
+	state = (struct concat_state){buf, 5u, 0u};
+	librecrypt_concat_memmove_(&state, buf, 4u);
+	assert(!memcmp(buf, "abcd", 5u));
+	assert(state.len == 4u);
+
+	buf[0u] = 'a';
+	buf[1u] = 'b';
+	buf[2u] = 'c';
+	buf[3u] = 'd';
+	buf[4u] = 'e';
+	state = (struct concat_state){buf, 5u, 0u};
+	librecrypt_concat_memmove_(&state, &buf[1u], 3u);
+	fprintf(stderr, "[[%s]]\n", buf);
+	assert(!memcmp(buf, "bcd", 3u));
+	assert(state.len == 3u);
+
+	STOP_RESOURCE_TEST();
+	return 0;
+}
+
+
+#endif
diff --git a/librecrypt_concat_str_.c b/librecrypt_concat_str_.c
index d89f4dd..90ac364 100644
--- a/librecrypt_concat_str_.c
+++ b/librecrypt_concat_str_.c
@@ -3,7 +3,7 @@
 #ifndef TEST
 
 
-extern inline void librecrypt_concat_str_(struct concat_state *state, const char *text);
+extern inline void librecrypt_concat_str_(struct concat_state *state, const char *restrict text);
 
 
 #else
diff --git a/librecrypt_hash_.c b/librecrypt_hash_.c
index 3f3bfea..04c589e 100644
--- a/librecrypt_hash_.c
+++ b/librecrypt_hash_.c
@@ -160,7 +160,7 @@ next:
 			goto einval;
 		/* Must align with fixed hash size when hash size is fixed */
 		if (!algo->flexible_hash_size && hash_size != algo->hash_size)
-			goto einval;
+			goto einval; /* TODO not covered */
 	}
 
 	/* For `librecrypt_crypt`: copy hash configurations to output */