Merge pull request #4509 from libgit2/ethomson/odb_alloc_error

odb: error when we can't alloc an object

6 files changed, 126 insertions, 53 deletions

diff --git a/src/hash/hash_win32.c b/src/hash/hash_win32.c
index 20ba9a5fe..50c23e816 100644
--- a/src/hash/hash_win32.c
+++ b/src/hash/hash_win32.c
@@ -24,16 +24,20 @@ GIT_INLINE(int) hash_cng_prov_init(void)
 	DWORD dll_path_len, size_len;
 
 	/* Only use CNG on Windows 2008 / Vista SP1  or better (Windows 6.0 SP1) */
-	if (!git_has_win32_version(6, 0, 1))
+	if (!git_has_win32_version(6, 0, 1)) {
+		giterr_set(GITERR_SHA1, "CryptoNG is not supported on this platform");
 		return -1;
+	}
 
 	/* Load bcrypt.dll explicitly from the system directory */
 	if ((dll_path_len = GetSystemDirectory(dll_path, MAX_PATH)) == 0 ||
 		dll_path_len > MAX_PATH ||
 		StringCchCat(dll_path, MAX_PATH, "\\") < 0 ||
 		StringCchCat(dll_path, MAX_PATH, GIT_HASH_CNG_DLL_NAME) < 0 ||
-		(hash_prov.prov.cng.dll = LoadLibrary(dll_path)) == NULL)
+		(hash_prov.prov.cng.dll = LoadLibrary(dll_path)) == NULL) {
+		giterr_set(GITERR_SHA1, "CryptoNG library could not be loaded");
 		return -1;
+	}
 
 	/* Load the function addresses */
 	if ((hash_prov.prov.cng.open_algorithm_provider = (hash_win32_cng_open_algorithm_provider_fn)GetProcAddress(hash_prov.prov.cng.dll, "BCryptOpenAlgorithmProvider")) == NULL ||
@@ -44,12 +48,16 @@ GIT_INLINE(int) hash_cng_prov_init(void)
 		(hash_prov.prov.cng.destroy_hash = (hash_win32_cng_destroy_hash_fn)GetProcAddress(hash_prov.prov.cng.dll, "BCryptDestroyHash")) == NULL ||
 		(hash_prov.prov.cng.close_algorithm_provider = (hash_win32_cng_close_algorithm_provider_fn)GetProcAddress(hash_prov.prov.cng.dll, "BCryptCloseAlgorithmProvider")) == NULL) {
 		FreeLibrary(hash_prov.prov.cng.dll);
+
+		giterr_set(GITERR_OS, "CryptoNG functions could not be loaded");
 		return -1;
 	}
 
 	/* Load the SHA1 algorithm */
 	if (hash_prov.prov.cng.open_algorithm_provider(&hash_prov.prov.cng.handle, GIT_HASH_CNG_HASH_TYPE, NULL, GIT_HASH_CNG_HASH_REUSABLE) < 0) {
 		FreeLibrary(hash_prov.prov.cng.dll);
+
+		giterr_set(GITERR_OS "algorithm provider could not be initialized");
 		return -1;
 	}
 
@@ -57,6 +65,8 @@ GIT_INLINE(int) hash_cng_prov_init(void)
 	if (hash_prov.prov.cng.get_property(hash_prov.prov.cng.handle, GIT_HASH_CNG_HASH_OBJECT_LEN, (PBYTE)&hash_prov.prov.cng.hash_object_size, sizeof(DWORD), &size_len, 0) < 0) {
 		hash_prov.prov.cng.close_algorithm_provider(hash_prov.prov.cng.handle, 0);
 		FreeLibrary(hash_prov.prov.cng.dll);
+
+		giterr_set(GITERR_OS, "algorithm handle could not be found");
 		return -1;
 	}
 
@@ -75,8 +85,10 @@ GIT_INLINE(void) hash_cng_prov_shutdown(void)
 /* Initialize CryptoAPI */
 GIT_INLINE(int) hash_cryptoapi_prov_init()
 {
-	if (!CryptAcquireContext(&hash_prov.prov.cryptoapi.handle, NULL, 0, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT))
+	if (!CryptAcquireContext(&hash_prov.prov.cryptoapi.handle, NULL, 0, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT)) {
+		giterr_set(GITERR_OS, "legacy hash context could not be started");
 		return -1;
+	}
 
 	hash_prov.type = CRYPTOAPI;
 	return 0;
@@ -129,6 +141,7 @@ GIT_INLINE(int) hash_cryptoapi_init(git_hash_ctx *ctx)
 
 	if (!CryptCreateHash(ctx->prov->prov.cryptoapi.handle, CALG_SHA1, 0, 0, &ctx->ctx.cryptoapi.hash_handle)) {
 		ctx->ctx.cryptoapi.valid = 0;
+		giterr_set(GITERR_OS, "legacy hash implementation could not be created");
 		return -1;
 	}
 
@@ -145,8 +158,10 @@ GIT_INLINE(int) hash_cryptoapi_update(git_hash_ctx *ctx, const void *_data, size
 	while (len > 0) {
 		DWORD chunk = (len > MAXDWORD) ? MAXDWORD : (DWORD)len;
 
-		if (!CryptHashData(ctx->ctx.cryptoapi.hash_handle, data, chunk, 0))
+		if (!CryptHashData(ctx->ctx.cryptoapi.hash_handle, data, chunk, 0)) {
+			giterr_set(GITERR_OS, "legacy hash data could not be updated");
 			return -1;
+		}
 
 		data += chunk;
 		len -= chunk;
@@ -162,8 +177,10 @@ GIT_INLINE(int) hash_cryptoapi_final(git_oid *out, git_hash_ctx *ctx)
 
 	assert(ctx->ctx.cryptoapi.valid);
 
-	if (!CryptGetHashParam(ctx->ctx.cryptoapi.hash_handle, HP_HASHVAL, out->id, &len, 0))
+	if (!CryptGetHashParam(ctx->ctx.cryptoapi.hash_handle, HP_HASHVAL, out->id, &len, 0)) {
+		giterr_set(GITERR_OS, "legacy hash data could not be finished");
 		error = -1;
+	}
 
 	CryptDestroyHash(ctx->ctx.cryptoapi.hash_handle);
 	ctx->ctx.cryptoapi.valid = 0;
@@ -186,6 +203,8 @@ GIT_INLINE(int) hash_ctx_cng_init(git_hash_ctx *ctx)
 
 	if (hash_prov.prov.cng.create_hash(hash_prov.prov.cng.handle, &ctx->ctx.cng.hash_handle, ctx->ctx.cng.hash_object, hash_prov.prov.cng.hash_object_size, NULL, 0, 0) < 0) {
 		git__free(ctx->ctx.cng.hash_object);
+
+		giterr_set(GITERR_OS, "hash implementation could not be created");
 		return -1;
 	}
 
@@ -203,8 +222,10 @@ GIT_INLINE(int) hash_cng_init(git_hash_ctx *ctx)
 		return 0;
 
 	/* CNG needs to be finished to restart */
-	if (ctx->prov->prov.cng.finish_hash(ctx->ctx.cng.hash_handle, hash, GIT_OID_RAWSZ, 0) < 0)
+	if (ctx->prov->prov.cng.finish_hash(ctx->ctx.cng.hash_handle, hash, GIT_OID_RAWSZ, 0) < 0) {
+		giterr_set(GITERR_OS, "hash implementation could not be finished");
 		return -1;
+	}
 
 	ctx->ctx.cng.updated = 0;
 
@@ -218,8 +239,10 @@ GIT_INLINE(int) hash_cng_update(git_hash_ctx *ctx, const void *_data, size_t len
 	while (len > 0) {
 		ULONG chunk = (len > ULONG_MAX) ? ULONG_MAX : (ULONG)len;
 
-		if (ctx->prov->prov.cng.hash_data(ctx->ctx.cng.hash_handle, data, chunk, 0) < 0)
+		if (ctx->prov->prov.cng.hash_data(ctx->ctx.cng.hash_handle, data, chunk, 0) < 0) {
+			giterr_set(GITERR_OS, "hash could not be updated");
 			return -1;
+		}
 
 		data += chunk;
 		len -= chunk;
@@ -230,8 +253,10 @@ GIT_INLINE(int) hash_cng_update(git_hash_ctx *ctx, const void *_data, size_t len
 
 GIT_INLINE(int) hash_cng_final(git_oid *out, git_hash_ctx *ctx)
 {
-	if (ctx->prov->prov.cng.finish_hash(ctx->ctx.cng.hash_handle, out->id, GIT_OID_RAWSZ, 0) < 0)
+	if (ctx->prov->prov.cng.finish_hash(ctx->ctx.cng.hash_handle, out->id, GIT_OID_RAWSZ, 0) < 0) {
+		giterr_set(GITERR_OS, "hash could not be finished");
 		return -1;
+	}
 
 	ctx->ctx.cng.updated = 0;
 
diff --git a/src/indexer.c b/src/indexer.c
index a5e842272..c0976f270 100644
--- a/src/indexer.c
+++ b/src/indexer.c
@@ -187,13 +187,17 @@ static int store_delta(git_indexer *idx)
 	return 0;
 }
 
-static void hash_header(git_hash_ctx *ctx, git_off_t len, git_otype type)
+static int hash_header(git_hash_ctx *ctx, git_off_t len, git_otype type)
 {
 	char buffer[64];
 	size_t hdrlen;
+	int error;
+
+	if ((error = git_odb__format_object_header(&hdrlen,
+		buffer, sizeof(buffer), (size_t)len, type)) < 0)
+		return error;
 
-	hdrlen = git_odb__format_object_header(buffer, sizeof(buffer), (size_t)len, type);
-	git_hash_update(ctx, buffer, hdrlen);
+	return git_hash_update(ctx, buffer, hdrlen);
 }
 
 static int hash_object_stream(git_indexer*idx, git_packfile_stream *stream)
@@ -621,7 +625,10 @@ int git_indexer_append(git_indexer *idx, const void *data, size_t size, git_tran
 				idx->have_delta = 1;
 			} else {
 				idx->have_delta = 0;
-				hash_header(&idx->hash_ctx, entry_size, type);
+
+				error = hash_header(&idx->hash_ctx, entry_size, type);
+				if (error < 0)
+					goto on_error;
 			}
 
 			idx->have_stream = 1;
diff --git a/src/odb.c b/src/odb.c
index 775cf90de..802c721a8 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -66,50 +66,75 @@ static git_otype odb_hardcoded_type(const git_oid *id)
 	return GIT_OBJ_BAD;
 }
 
-static int odb_read_hardcoded(git_rawobj *raw, const git_oid *id)
+static int odb_read_hardcoded(bool *found, git_rawobj *raw, const git_oid *id)
 {
-	git_otype type = odb_hardcoded_type(id);
-	if (type == GIT_OBJ_BAD)
-		return -1;
+	git_otype type;
+
+	*found = false;
+
+	if ((type = odb_hardcoded_type(id)) == GIT_OBJ_BAD)
+		return 0;
 
 	raw->type = type;
 	raw->len = 0;
 	raw->data = git__calloc(1, sizeof(uint8_t));
+	GITERR_CHECK_ALLOC(raw->data);
+
+	*found = true;
 	return 0;
 }
 
-int git_odb__format_object_header(char *hdr, size_t n, git_off_t obj_len, git_otype obj_type)
+int git_odb__format_object_header(
+	size_t *written,
+	char *hdr,
+	size_t hdr_size,
+	git_off_t obj_len,
+	git_otype obj_type)
 {
 	const char *type_str = git_object_type2string(obj_type);
-	int len = p_snprintf(hdr, n, "%s %lld", type_str, (long long)obj_len);
-	assert(len > 0 && len <= (int)n);
-	return len+1;
+	int hdr_max = (hdr_size > INT_MAX-2) ? (INT_MAX-2) : (int)hdr_size;
+	int len;
+
+	len = p_snprintf(hdr, hdr_max, "%s %lld", type_str, (long long)obj_len);
+
+	if (len < 0 || len >= hdr_max) {
+		giterr_set(GITERR_OS, "object header creation failed");
+		return -1;
+	}
+
+	*written = (size_t)(len + 1);
+	return 0;
 }
 
 int git_odb__hashobj(git_oid *id, git_rawobj *obj)
 {
 	git_buf_vec vec[2];
 	char header[64];
-	int hdrlen;
+	size_t hdrlen;
+	int error;
 
 	assert(id && obj);
 
-	if (!git_object_typeisloose(obj->type))
+	if (!git_object_typeisloose(obj->type)) {
+		giterr_set(GITERR_INVALID, "invalid object type");
 		return -1;
+	}
 
-	if (!obj->data && obj->len != 0)
+	if (!obj->data && obj->len != 0) {
+		giterr_set(GITERR_INVALID, "invalid object");
 		return -1;
+	}
 
-	hdrlen = git_odb__format_object_header(header, sizeof(header), obj->len, obj->type);
+	if ((error = git_odb__format_object_header(&hdrlen,
+		header, sizeof(header), obj->len, obj->type)) < 0)
+		return error;
 
 	vec[0].data = header;
 	vec[0].len = hdrlen;
 	vec[1].data = obj->data;
 	vec[1].len = obj->len;
 
-	git_hash_vec(id, vec, 2);
-
-	return 0;
+	return git_hash_vec(id, vec, 2);
 }
 
 
@@ -172,7 +197,7 @@ void git_odb_object_free(git_odb_object *object)
 
 int git_odb__hashfd(git_oid *out, git_file fd, size_t size, git_otype type)
 {
-	int hdr_len;
+	size_t hdr_len;
 	char hdr[64], buffer[FILEIO_BUFSIZE];
 	git_hash_ctx ctx;
 	ssize_t read_len = 0;
@@ -184,9 +209,11 @@ int git_odb__hashfd(git_oid *out, git_file fd, size_t size, git_otype type)
 	}
 
 	if ((error = git_hash_ctx_init(&ctx)) < 0)
-		return -1;
+		return error;
 
-	hdr_len = git_odb__format_object_header(hdr, sizeof(hdr), size, type);
+	if ((error = git_odb__format_object_header(&hdr_len, hdr,
+		sizeof(hdr), size, type)) < 0)
+		goto done;
 
 	if ((error = git_hash_update(&ctx, hdr, hdr_len)) < 0)
 		goto done;
@@ -342,8 +369,7 @@ static int fake_wstream__write(git_odb_stream *_stream, const char *data, size_t
 {
 	fake_wstream *stream = (fake_wstream *)_stream;
 
-	if (stream->written + len > stream->size)
-		return -1;
+	assert(stream->written + len > stream->size);
 
 	memcpy(stream->buffer + stream->written, data, len);
 	stream->written += len;
@@ -800,7 +826,7 @@ int git_odb_exists_prefix(
 	git_oid *out, git_odb *db, const git_oid *short_id, size_t len)
 {
 	int error;
-	git_oid key = {{0}}; 
+	git_oid key = {{0}};
 
 	assert(db && short_id);
 
@@ -1012,8 +1038,10 @@ static int odb_read_1(git_odb_object **out, git_odb *db, const git_oid *id,
 	bool found = false;
 	int error = 0;
 
-	if (!only_refreshed && odb_read_hardcoded(&raw, id) == 0)
-		found = true;
+	if (!only_refreshed) {
+		if ((error = odb_read_hardcoded(&found, &raw, id)) < 0)
+			return error;
+	}
 
 	for (i = 0; i < db->backends.length && !found; ++i) {
 		backend_internal *internal = git_vector_get(&db->backends, i);
@@ -1048,8 +1076,10 @@ static int odb_read_1(git_odb_object **out, git_odb *db, const git_oid *id,
 	}
 
 	giterr_clear();
-	if ((object = odb_object__alloc(id, &raw)) == NULL)
+	if ((object = odb_object__alloc(id, &raw)) == NULL) {
+		error = -1;
 		goto out;
+	}
 
 	*out = git_cache_store_raw(odb_cache(db), object);
 
@@ -1096,7 +1126,7 @@ static int odb_otype_fast(git_otype *type_p, git_odb *db, const git_oid *id)
 		*type_p = object->cached.type;
 		return 0;
 	}
-	
+
 	error = odb_read_header_1(&_unused, type_p, db, id, false);
 
 	if (error == GIT_PASSTHROUGH) {
@@ -1175,8 +1205,10 @@ static int read_prefix_1(git_odb_object **out, git_odb *db,
 		}
 	}
 
-	if ((object = odb_object__alloc(&found_full_oid, &raw)) == NULL)
+	if ((object = odb_object__alloc(&found_full_oid, &raw)) == NULL) {
+		error = -1;
 		goto out;
+	}
 
 	*out = git_cache_store_raw(odb_cache(db), object);
 
@@ -1281,13 +1313,17 @@ int git_odb_write(
 	return error;
 }
 
-static void hash_header(git_hash_ctx *ctx, git_off_t size, git_otype type)
+static int hash_header(git_hash_ctx *ctx, git_off_t size, git_otype type)
 {
 	char header[64];
-	int hdrlen;
+	size_t hdrlen;
+	int error;
+
+	 if ((error = git_odb__format_object_header(&hdrlen,
+		header, sizeof(header), size, type)) < 0)
+		return error;
 
-	hdrlen = git_odb__format_object_header(header, sizeof(header), size, type);
-	git_hash_update(ctx, header, hdrlen);
+	return git_hash_update(ctx, header, hdrlen);
 }
 
 int git_odb_open_wstream(
@@ -1328,12 +1364,11 @@ int git_odb_open_wstream(
 	ctx = git__malloc(sizeof(git_hash_ctx));
 	GITERR_CHECK_ALLOC(ctx);
 
-	if ((error = git_hash_ctx_init(ctx)) < 0)
+	if ((error = git_hash_ctx_init(ctx)) < 0 ||
+		(error = hash_header(ctx, size, type)) < 0)
 		goto done;
 
-	hash_header(ctx, size, type);
 	(*stream)->hash_ctx = ctx;
-
 	(*stream)->declared_size = size;
 	(*stream)->received_bytes = 0;
 
diff --git a/src/odb.h b/src/odb.h
index 6845b22f1..b354108e7 100644
--- a/src/odb.h
+++ b/src/odb.h
@@ -70,7 +70,7 @@ int git_odb__hashobj(git_oid *id, git_rawobj *obj);
 /*
  * Format the object header such as it would appear in the on-disk object
  */
-int git_odb__format_object_header(char *hdr, size_t n, git_off_t obj_len, git_otype obj_type);
+int git_odb__format_object_header(size_t *out_len, char *hdr, size_t hdr_size, git_off_t obj_len, git_otype obj_type);
 /*
  * Hash an open file descriptor.
  * This is a performance call when the contents of a fd need to be hashed,
diff --git a/src/odb_loose.c b/src/odb_loose.c
index 7d77eed38..713288da2 100644
--- a/src/odb_loose.c
+++ b/src/odb_loose.c
@@ -824,14 +824,17 @@ static int loose_backend__writestream(git_odb_stream **stream_out, git_odb_backe
 	loose_writestream *stream = NULL;
 	char hdr[MAX_HEADER_LEN];
 	git_buf tmp_path = GIT_BUF_INIT;
-	int hdrlen;
+	size_t hdrlen;
+	int error;
 
 	assert(_backend && length >= 0);
 
 	backend = (loose_backend *)_backend;
 	*stream_out = NULL;
 
-	hdrlen = git_odb__format_object_header(hdr, sizeof(hdr), length, type);
+	if ((error = git_odb__format_object_header(&hdrlen,
+		hdr, sizeof(hdr), length, type)) < 0)
+		return error;
 
 	stream = git__calloc(1, sizeof(loose_writestream));
 	GITERR_CHECK_ALLOC(stream);
@@ -1036,16 +1039,19 @@ done:
 
 static int loose_backend__write(git_odb_backend *_backend, const git_oid *oid, const void *data, size_t len, git_otype type)
 {
-	int error = 0, header_len;
+	int error = 0;
 	git_buf final_path = GIT_BUF_INIT;
 	char header[MAX_HEADER_LEN];
+	size_t header_len;
 	git_filebuf fbuf = GIT_FILEBUF_INIT;
 	loose_backend *backend;
 
 	backend = (loose_backend *)_backend;
 
 	/* prepare the header for the file */
-	header_len = git_odb__format_object_header(header, sizeof(header), len, type);
+	if ((error = git_odb__format_object_header(&header_len,
+		header, sizeof(header), len, type)) < 0)
+		goto cleanup;
 
 	if (git_buf_joinpath(&final_path, backend->objects_dir, "tmp_object") < 0 ||
 		git_filebuf_open(&fbuf, final_path.ptr, filebuf_flags(backend),
diff --git a/tests/odb/largefiles.c b/tests/odb/largefiles.c
index cd3651b4a..a5b982e14 100644
--- a/tests/odb/largefiles.c
+++ b/tests/odb/largefiles.c
@@ -87,10 +87,10 @@ void test_odb_largefiles__streamread(void)
 	git_odb_stream *stream;
 	char buf[10240];
 	char hdr[64];
-	size_t len, total = 0;
+	size_t len, hdr_len, total = 0;
 	git_hash_ctx hash;
 	git_otype type;
-	int hdr_len, ret;
+	int ret;
 
 #ifndef GIT_ARCH_64
 	cl_skip();
@@ -108,7 +108,7 @@ void test_odb_largefiles__streamread(void)
 	cl_assert_equal_i(GIT_OBJ_BLOB, type);
 
 	cl_git_pass(git_hash_ctx_init(&hash));
-	hdr_len = git_odb__format_object_header(hdr, sizeof(hdr), len, type);
+	cl_git_pass(git_odb__format_object_header(&hdr_len, hdr, sizeof(hdr), len, type));
 
 	cl_git_pass(git_hash_update(&hash, hdr, hdr_len));