buffer: git_buf_sanitize should return a value

`git_buf_sanitize` is called with user-input, and wants to sanity-check
that input.  Allow it to return a value if the input was malformed in a
way that we cannot cope.

17 files changed, 133 insertions, 61 deletions

diff --git a/src/blob.c b/src/blob.c
index 392a1fe07..97069645c 100644
--- a/src/blob.c
+++ b/src/blob.c
@@ -423,11 +423,12 @@ int git_blob_filter(
 	GIT_ASSERT_ARG(path);
 	GIT_ASSERT_ARG(out);
 
-	git_buf_sanitize(out);
-
 	GIT_ERROR_CHECK_VERSION(
 		given_opts, GIT_BLOB_FILTER_OPTIONS_VERSION, "git_blob_filter_options");
 
+	if (git_buf_sanitize(out) < 0)
+		return -1;
+
 	if (given_opts != NULL)
 		memcpy(&opts, given_opts, sizeof(git_blob_filter_options));
 
diff --git a/src/branch.c b/src/branch.c
index 08fd95d58..29ff0b9d9 100644
--- a/src/branch.c
+++ b/src/branch.c
@@ -417,7 +417,8 @@ int git_branch_upstream_name(
 	GIT_ASSERT_ARG(out);
 	GIT_ASSERT_ARG(refname);
 
-	git_buf_sanitize(out);
+	if ((error = git_buf_sanitize(out)) < 0)
+		return error;
 
 	if (!git_reference__is_branch(refname))
 		return not_a_local_branch(refname);
@@ -478,9 +479,8 @@ int git_branch_upstream_remote(git_buf *buf, git_repository *repo, const char *r
 	if ((error = git_repository_config__weakptr(&cfg, repo)) < 0)
 		return error;
 
-	git_buf_sanitize(buf);
-
-	if ((error = retrieve_upstream_configuration(buf, cfg, refname, "branch.%s.remote")) < 0)
+	if ((error = git_buf_sanitize(buf)) < 0 ||
+	    (error = retrieve_upstream_configuration(buf, cfg, refname, "branch.%s.remote")) < 0)
 		return error;
 
 	if (git_buf_len(buf) == 0) {
@@ -505,7 +505,8 @@ int git_branch_remote_name(git_buf *buf, git_repository *repo, const char *refna
 	GIT_ASSERT_ARG(repo);
 	GIT_ASSERT_ARG(refname);
 
-	git_buf_sanitize(buf);
+	if ((error = git_buf_sanitize(buf)) < 0)
+		return error;
 
 	/* Verify that this is a remote branch */
 	if (!git_reference__is_remote(refname)) {
diff --git a/src/buffer.c b/src/buffer.c
index f395a77cc..2928b1767 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -140,13 +140,17 @@ void git_buf_free(git_buf *buf)
 }
 #endif
 
-void git_buf_sanitize(git_buf *buf)
+int git_buf_sanitize(git_buf *buf)
 {
 	if (buf->ptr == NULL) {
-		assert(buf->size == 0 && buf->asize == 0);
+		GIT_ASSERT_ARG(buf->size == 0 && buf->asize == 0);
+
 		buf->ptr = git_buf__initbuf;
-	} else if (buf->asize > buf->size)
+	} else if (buf->asize > buf->size) {
 		buf->ptr[buf->size] = '\0';
+	}
+
+	return 0;
 }
 
 void git_buf_clear(git_buf *buf)
diff --git a/src/buffer.h b/src/buffer.h
index 6b717d2e9..6257b435b 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -69,7 +69,7 @@ extern int git_buf_try_grow(
  * git_buf__initbuf. If a buffer with a non-NULL ptr is passed in, this method
  * assures that the buffer is '\0'-terminated.
  */
-extern void git_buf_sanitize(git_buf *buf);
+extern int git_buf_sanitize(git_buf *buf);
 
 extern void git_buf_swap(git_buf *buf_a, git_buf *buf_b);
 extern char *git_buf_detach(git_buf *buf);
diff --git a/src/config.c b/src/config.c
index 42f46e6cf..b3e7324a1 100644
--- a/src/config.c
+++ b/src/config.c
@@ -886,7 +886,8 @@ int git_config_get_string_buf(
 	int ret;
 	const char *str;
 
-	git_buf_sanitize(out);
+	if ((ret = git_buf_sanitize(out)) < 0)
+		return ret;
 
 	ret  = get_entry(&entry, cfg, name, true, GET_ALL_ERRORS);
 	str = !ret ? (entry->value ? entry->value : "") : NULL;
@@ -1084,19 +1085,31 @@ void git_config_iterator_free(git_config_iterator *iter)
 
 int git_config_find_global(git_buf *path)
 {
-	git_buf_sanitize(path);
+	int error;
+
+	if ((error = git_buf_sanitize(path)) < 0)
+		return error;
+
 	return git_sysdir_find_global_file(path, GIT_CONFIG_FILENAME_GLOBAL);
 }
 
 int git_config_find_xdg(git_buf *path)
 {
-	git_buf_sanitize(path);
+	int error;
+
+	if ((error = git_buf_sanitize(path)) < 0)
+		return error;
+
 	return git_sysdir_find_xdg_file(path, GIT_CONFIG_FILENAME_XDG);
 }
 
 int git_config_find_system(git_buf *path)
 {
-	git_buf_sanitize(path);
+	int error;
+
+	if ((error = git_buf_sanitize(path)) < 0)
+		return error;
+
 	return git_sysdir_find_system_file(path, GIT_CONFIG_FILENAME_SYSTEM);
 }
 
@@ -1104,7 +1117,9 @@ int git_config_find_programdata(git_buf *path)
 {
 	int ret;
 
-	git_buf_sanitize(path);
+	if ((ret = git_buf_sanitize(path)) < 0)
+		return ret;
+
 	ret = git_sysdir_find_programdata_file(path,
 					       GIT_CONFIG_FILENAME_PROGRAMDATA);
 	if (ret != GIT_OK)
@@ -1360,9 +1375,12 @@ fail_parse:
 
 int git_config_parse_path(git_buf *out, const char *value)
 {
+	int error;
+
 	assert(out && value);
 
-	git_buf_sanitize(out);
+	if ((error = git_buf_sanitize(out)) < 0)
+		return error;
 
 	if (value[0] == '~') {
 		if (value[1] != '\0' && value[1] != '/') {
diff --git a/src/describe.c b/src/describe.c
index 8beffdeba..ebf70b062 100644
--- a/src/describe.c
+++ b/src/describe.c
@@ -780,7 +780,8 @@ int git_describe_format(git_buf *out, const git_describe_result *result, const g
 	GIT_ERROR_CHECK_VERSION(given, GIT_DESCRIBE_FORMAT_OPTIONS_VERSION, "git_describe_format_options");
 	normalize_format_options(&opts, given);
 
-	git_buf_sanitize(out);
+	if ((error = git_buf_sanitize(out)) < 0)
+		return error;
 
 
 	if (opts.always_use_long_format && opts.abbreviated_size == 0) {
diff --git a/src/diff_print.c b/src/diff_print.c
index 9ce04d70e..0e93fe893 100644
--- a/src/diff_print.c
+++ b/src/diff_print.c
@@ -764,8 +764,12 @@ int git_diff_print_callback__to_file_handle(
 /* print a git_diff to a git_buf */
 int git_diff_to_buf(git_buf *out, git_diff *diff, git_diff_format_t format)
 {
+	int error;
+
 	assert(out && diff);
-	git_buf_sanitize(out);
+	if ((error = git_buf_sanitize(out)) < 0)
+		return error;
+
 	return git_diff_print(diff, format, git_diff_print_callback__to_buf, out);
 }
 
@@ -799,7 +803,12 @@ out:
 /* print a git_patch to a git_buf */
 int git_patch_to_buf(git_buf *out, git_patch *patch)
 {
+	int error;
+
 	assert(out && patch);
-	git_buf_sanitize(out);
+
+	if ((error = git_buf_sanitize(out)) < 0)
+		return error;
+
 	return git_patch_print(patch, git_diff_print_callback__to_buf, out);
 }
diff --git a/src/filter.c b/src/filter.c
index 86db5a5e3..ae9220c9b 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -725,8 +725,9 @@ int git_filter_list_apply_to_data(
 	struct buf_stream writer;
 	int error;
 
-	git_buf_sanitize(tgt);
-	git_buf_sanitize(src);
+	if ((error = git_buf_sanitize(tgt)) < 0 ||
+	    (error = git_buf_sanitize(src)) < 0)
+	    return error;
 
 	if (!filters) {
 		git_buf_attach_notowned(tgt, src->ptr, src->size);
@@ -832,7 +833,9 @@ static int proxy_stream_close(git_writestream *s)
 	if (error == GIT_PASSTHROUGH) {
 		writebuf = &proxy_stream->input;
 	} else if (error == 0) {
-		git_buf_sanitize(proxy_stream->output);
+		if ((error = git_buf_sanitize(proxy_stream->output)) < 0)
+			return error;
+
 		writebuf = proxy_stream->output;
 	} else {
 		/* close stream before erroring out taking care
@@ -1004,7 +1007,8 @@ int git_filter_list_stream_data(
 	git_writestream *stream_start;
 	int error, initialized = 0;
 
-	git_buf_sanitize(data);
+	if ((error = git_buf_sanitize(data)) < 0)
+		return error;
 
 	if ((error = stream_list_init(&stream_start, &filter_streams, filters, target)) < 0)
 		goto out;
diff --git a/src/libgit2.c b/src/libgit2.c
index 07414bec8..9e5112dbf 100644
--- a/src/libgit2.c
+++ b/src/libgit2.c
@@ -122,29 +122,28 @@ int git_libgit2_features(void)
 	;
 }
 
-static int config_level_to_sysdir(int config_level)
+static int config_level_to_sysdir(int *out, int config_level)
 {
-	int val = -1;
-
 	switch (config_level) {
 	case GIT_CONFIG_LEVEL_SYSTEM:
-		val = GIT_SYSDIR_SYSTEM;
-		break;
+		*out = GIT_SYSDIR_SYSTEM;
+		return 0;
 	case GIT_CONFIG_LEVEL_XDG:
-		val = GIT_SYSDIR_XDG;
-		break;
+		*out = GIT_SYSDIR_XDG;
+		return 0;
 	case GIT_CONFIG_LEVEL_GLOBAL:
-		val = GIT_SYSDIR_GLOBAL;
-		break;
+		*out = GIT_SYSDIR_GLOBAL;
+		return 0;
 	case GIT_CONFIG_LEVEL_PROGRAMDATA:
-		val = GIT_SYSDIR_PROGRAMDATA;
-		break;
+		*out = GIT_SYSDIR_PROGRAMDATA;
+		return 0;
 	default:
-		git_error_set(
-			GIT_ERROR_INVALID, "invalid config path selector %d", config_level);
+		break;
 	}
 
-	return val;
+	git_error_set(
+		GIT_ERROR_INVALID, "invalid config path selector %d", config_level);
+	return -1;
 }
 
 const char *git_libgit2__user_agent(void)
@@ -190,12 +189,15 @@ int git_libgit2_opts(int key, ...)
 		break;
 
 	case GIT_OPT_GET_SEARCH_PATH:
-		if ((error = config_level_to_sysdir(va_arg(ap, int))) >= 0) {
+		{
+			int sysdir = va_arg(ap, int);
 			git_buf *out = va_arg(ap, git_buf *);
 			const git_buf *tmp;
+			int level;
 
-			git_buf_sanitize(out);
-			if ((error = git_sysdir_get(&tmp, error)) < 0)
+			if ((error = config_level_to_sysdir(&level, sysdir)) < 0 ||
+			    (error = git_buf_sanitize(out)) < 0 ||
+			    (error = git_sysdir_get(&tmp, level)) < 0)
 				break;
 
 			error = git_buf_sets(out, tmp->ptr);
@@ -203,8 +205,12 @@ int git_libgit2_opts(int key, ...)
 		break;
 
 	case GIT_OPT_SET_SEARCH_PATH:
-		if ((error = config_level_to_sysdir(va_arg(ap, int))) >= 0)
-			error = git_sysdir_set(error, va_arg(ap, const char *));
+		{
+			int level;
+
+			if ((error = config_level_to_sysdir(&level, va_arg(ap, int))) >= 0)
+				error = git_sysdir_set(level, va_arg(ap, const char *));
+		}
 		break;
 
 	case GIT_OPT_SET_CACHE_OBJECT_LIMIT:
@@ -233,8 +239,8 @@ int git_libgit2_opts(int key, ...)
 			git_buf *out = va_arg(ap, git_buf *);
 			const git_buf *tmp;
 
-			git_buf_sanitize(out);
-			if ((error = git_sysdir_get(&tmp, GIT_SYSDIR_TEMPLATE)) < 0)
+			if ((error = git_buf_sanitize(out)) < 0 ||
+			    (error = git_sysdir_get(&tmp, GIT_SYSDIR_TEMPLATE)) < 0)
 				break;
 
 			error = git_buf_sets(out, tmp->ptr);
@@ -303,7 +309,8 @@ int git_libgit2_opts(int key, ...)
 	case GIT_OPT_GET_USER_AGENT:
 		{
 			git_buf *out = va_arg(ap, git_buf *);
-			git_buf_sanitize(out);
+			if ((error = git_buf_sanitize(out)) < 0)
+				break;
 			error = git_buf_sets(out, git__user_agent);
 		}
 		break;
diff --git a/src/message.c b/src/message.c
index 6c5a2379f..327b984fc 100644
--- a/src/message.c
+++ b/src/message.c
@@ -28,8 +28,10 @@ int git_message_prettify(git_buf *message_out, const char *message, int strip_co
 	int consecutive_empty_lines = 0;
 	size_t i, line_length, rtrimmed_line_length;
 	char *next_newline;
+	int error;
 
-	git_buf_sanitize(message_out);
+	if ((error = git_buf_sanitize(message_out)) < 0)
+		return error;
 
 	for (i = 0; i < strlen(message); i += line_length) {
 		next_newline = memchr(message + i, '\n', message_len - i);
diff --git a/src/notes.c b/src/notes.c
index 68d2ae9ec..a3c4b7c4f 100644
--- a/src/notes.c
+++ b/src/notes.c
@@ -629,9 +629,8 @@ int git_note_default_ref(git_buf *out, git_repository *repo)
 
 	assert(out && repo);
 
-	git_buf_sanitize(out);
-
-	if ((error = note_get_default_ref(&default_ref, repo)) < 0)
+	if ((error = git_buf_sanitize(out)) < 0 ||
+	    (error = note_get_default_ref(&default_ref, repo)) < 0)
 		return error;
 
 	git_buf_attach(out, default_ref, strlen(default_ref));
diff --git a/src/object.c b/src/object.c
index 749b0caf2..fa975347e 100644
--- a/src/object.c
+++ b/src/object.c
@@ -495,7 +495,9 @@ int git_object_short_id(git_buf *out, const git_object *obj)
 
 	assert(out && obj);
 
-	git_buf_sanitize(out);
+	if ((error = git_buf_sanitize(out)) < 0)
+		return error;
+
 	repo = git_object_owner(obj);
 
 	if ((error = git_repository__configmap_lookup(&len, repo, GIT_CONFIGMAP_ABBREV)) < 0)
diff --git a/src/pack-objects.c b/src/pack-objects.c
index cf10e6cb5..9679e8e23 100644
--- a/src/pack-objects.c
+++ b/src/pack-objects.c
@@ -1363,8 +1363,13 @@ int git_packbuilder_foreach(git_packbuilder *pb, int (*cb)(void *buf, size_t siz
 
 int git_packbuilder_write_buf(git_buf *buf, git_packbuilder *pb)
 {
+	int error;
+
+	if ((error = git_buf_sanitize(buf)) < 0)
+		return error;
+
 	PREPARE_PACK;
-	git_buf_sanitize(buf);
+
 	return write_pack(pb, &write_pack_buf, buf);
 }
 
diff --git a/src/refspec.c b/src/refspec.c
index 4245cbbda..a778f62b4 100644
--- a/src/refspec.c
+++ b/src/refspec.c
@@ -245,8 +245,11 @@ static int refspec_transform(
 {
 	const char *from_star, *to_star;
 	size_t replacement_len, star_offset;
+	int error;
+
+	if ((error = git_buf_sanitize(out)) < 0)
+		return error;
 
-	git_buf_sanitize(out);
 	git_buf_clear(out);
 
 	/*
@@ -278,8 +281,12 @@ static int refspec_transform(
 
 int git_refspec_transform(git_buf *out, const git_refspec *spec, const char *name)
 {
+	int error;
+
 	assert(out && spec && name);
-	git_buf_sanitize(out);
+
+	if ((error = git_buf_sanitize(out)) < 0)
+		return error;
 
 	if (!git_refspec_src_matches(spec, name)) {
 		git_error_set(GIT_ERROR_INVALID, "ref '%s' doesn't match the source", name);
@@ -294,8 +301,12 @@ int git_refspec_transform(git_buf *out, const git_refspec *spec, const char *nam
 
 int git_refspec_rtransform(git_buf *out, const git_refspec *spec, const char *name)
 {
+	int error;
+
 	assert(out && spec && name);
-	git_buf_sanitize(out);
+
+	if ((error = git_buf_sanitize(out)) < 0)
+		return error;
 
 	if (!git_refspec_dst_matches(spec, name)) {
 		git_error_set(GIT_ERROR_INVALID, "ref '%s' doesn't match the destination", name);
diff --git a/src/remote.c b/src/remote.c
index f63824861..a12fa00ba 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -648,14 +648,17 @@ int git_remote_set_pushurl(git_repository *repo, const char *remote, const char*
 
 static int resolve_url(git_buf *resolved_url, const char *url, int direction, const git_remote_callbacks *callbacks)
 {
-	int status;
+	int status, error;
 
 	if (callbacks && callbacks->resolve_url) {
 		git_buf_clear(resolved_url);
 		status = callbacks->resolve_url(resolved_url, url, direction, callbacks->payload);
 		if (status != GIT_PASSTHROUGH) {
 			git_error_set_after_callback_function(status, "git_resolve_url_cb");
-			git_buf_sanitize(resolved_url);
+
+			if ((error = git_buf_sanitize(resolved_url)) < 0)
+				return error;
+
 			return status;
 		}
 	}
@@ -2403,7 +2406,8 @@ int git_remote_default_branch(git_buf *out, git_remote *remote)
 		goto done;
 	}
 
-	git_buf_sanitize(out);
+	if ((error = git_buf_sanitize(out)) < 0)
+		return error;
 
 	/* the first one must be HEAD so if that has the symref info, we're done */
 	if (heads[0]->symref_target) {
diff --git a/src/repository.c b/src/repository.c
index f0d4b06aa..28cbf3c6c 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -945,10 +945,12 @@ int git_repository_discover(
 	const char *ceiling_dirs)
 {
 	uint32_t flags = across_fs ? GIT_REPOSITORY_OPEN_CROSS_FS : 0;
+	int error;
 
 	assert(start_path);
 
-	git_buf_sanitize(out);
+	if ((error = git_buf_sanitize(out)) < 0)
+		return error;
 
 	return find_repo(out, NULL, NULL, NULL, start_path, flags, ceiling_dirs);
 }
@@ -2609,7 +2611,8 @@ int git_repository_message(git_buf *out, git_repository *repo)
 	struct stat st;
 	int error;
 
-	git_buf_sanitize(out);
+	if ((error = git_buf_sanitize(out)) < 0)
+		return error;
 
 	if (git_buf_joinpath(&path, repo->gitdir, GIT_MERGE_MSG_FILE) < 0)
 		return -1;
diff --git a/src/submodule.c b/src/submodule.c
index c32b9770a..8fb3bdb73 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -998,7 +998,8 @@ int git_submodule_resolve_url(git_buf *out, git_repository *repo, const char *ur
 
 	assert(out && repo && url);
 
-	git_buf_sanitize(out);
+	if ((error = git_buf_sanitize(out)) < 0)
+		return error;
 
 	/* We do this in all platforms in case someone on Windows created the .gitmodules */
 	if (strchr(url, '\\')) {