[pacman-dev] [PATCH] Fix memory leak in download payload->remote_name

[Dan McGee]

In the sync code, we explicitly allocated a string for this field, while
in the dload code itself it was filled in with a pointer to another
string. This led to a memory leak in the sync download case.

Make remote_name non-const and always explicitly allocate it. This patch
ensures this as well as uses malloc + snprintf (rather than calloc) in
several codepaths, and eliminates the only use of PATH_MAX in the
download code.

Signed-off-by: Dan McGee <dan at archlinux.org>
---
 lib/libalpm/dload.c |   26 ++++++++++++++++----------
 lib/libalpm/dload.h |    2 +-
 lib/libalpm/sync.c  |    2 +-
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 6f0139d..1e8db1e 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -60,7 +60,7 @@ static char *get_fullpath(const char *path, const char *filename,
 	char *filepath;
 	/* len = localpath len + filename len + suffix len + null */
 	size_t len = strlen(path) + strlen(filename) + strlen(suffix) + 1;
-	CALLOC(filepath, len, sizeof(char), return NULL);
+	MALLOC(filepath, len, return NULL);
 	snprintf(filepath, len, "%s%s%s", path, filename, suffix);
 
 	return filepath;
@@ -279,22 +279,27 @@ static FILE *create_tempfile(struct dload_payload *payload, const char *localpat
 {
 	int fd;
 	FILE *fp;
-	char randpath[PATH_MAX];
-	alpm_handle_t *handle = payload->handle;
+	char *randpath;
+	size_t len;
 
 	/* create a random filename, which is opened with O_EXCL */
-	snprintf(randpath, PATH_MAX, "%salpmtmp.XXXXXX", localpath);
+	len = strlen(localpath) + 14 + 1;
+	MALLOC(randpath, len, RET_ERR(payload->handle, ALPM_ERR_MEMORY, NULL));
+	snprintf(randpath, len, "%salpmtmp.XXXXXX", localpath);
 	if((fd = mkstemp(randpath)) == -1 ||
 			!(fp = fdopen(fd, payload->tempfile_openmode))) {
 		unlink(randpath);
 		close(fd);
-		_alpm_log(handle, ALPM_LOG_ERROR,
+		_alpm_log(payload->handle, ALPM_LOG_ERROR,
 				_("failed to create temporary file for download\n"));
 		return NULL;
 	}
 	/* fp now points to our alpmtmp.XXXXXX */
-	STRDUP(payload->tempfile_name, randpath, RET_ERR(handle, ALPM_ERR_MEMORY, NULL));
-	payload->remote_name = strrchr(randpath, '/') + 1;
+	free(payload->tempfile_name);
+	payload->tempfile_name = randpath;
+	free(payload->remote_name);
+	STRDUP(payload->remote_name, strrchr(randpath, '/') + 1,
+			RET_ERR(payload->handle, ALPM_ERR_MEMORY, NULL));
 
 	return fp;
 }
@@ -318,7 +323,7 @@ static int curl_download_internal(struct dload_payload *payload,
 
 	payload->tempfile_openmode = "wb";
 	if(!payload->remote_name) {
-		payload->remote_name = get_filename(payload->fileurl);
+		payload->remote_name = strdup(get_filename(payload->fileurl));
 	}
 	if(!payload->remote_name || curl_gethost(payload->fileurl, hostname) != 0) {
 		_alpm_log(handle, ALPM_LOG_ERROR, _("url '%s' is invalid\n"), payload->fileurl);
@@ -591,10 +596,11 @@ char SYMEXPORT *alpm_fetch_pkgurl(alpm_handle_t *handle, const char *url)
 void _alpm_dload_payload_free(struct dload_payload *payload) {
 	ASSERT(payload, return);
 
-	FREE(payload->fileurl);
-	FREE(payload->content_disp_name);
+	FREE(payload->remote_name);
 	FREE(payload->tempfile_name);
 	FREE(payload->destfile_name);
+	FREE(payload->content_disp_name);
+	FREE(payload->fileurl);
 	FREE(payload);
 
 }
diff --git a/lib/libalpm/dload.h b/lib/libalpm/dload.h
index bc5eb13..765c5fd 100644
--- a/lib/libalpm/dload.h
+++ b/lib/libalpm/dload.h
@@ -27,8 +27,8 @@
 
 struct dload_payload {
 	alpm_handle_t *handle;
-	const char *remote_name;
 	const char *tempfile_openmode;
+	char *remote_name;
 	char *tempfile_name;
 	char *destfile_name;
 	char *content_disp_name;
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index 04ea7d2..8c50ec8 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -874,7 +874,7 @@ static int download_files(alpm_handle_t *handle, alpm_list_t **deltas)
 
 					/* print server + filename into a buffer */
 					len = strlen(server_url) + strlen(payload->remote_name) + 2;
-					CALLOC(payload->fileurl, len, sizeof(char), RET_ERR(handle, ALPM_ERR_MEMORY, -1));
+					MALLOC(payload->fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1));
 					snprintf(payload->fileurl, len, "%s/%s", server_url, payload->remote_name);
 					payload->handle = handle;
 					payload->allow_resume = 1;
-- 
1.7.6.4