[pacman-dev] [PATCH] curl_gethost() potential bug fixups

[Dan McGee]

This is in the realm of "probably not going to happen", but if someone
were to translate "disk" to a string longer than 256 characters, we
would have a smashed/corrupted stack due to our unchecked strcpy() call.
Rework the function to always length-check the value we copy into the
hostname buffer, and do it with memcpy rather than the more cumbersome
and unnecessary snprintf.

Finally, move the magic 256 value into a constant and pass it into the
function which is going to get inlined anyway.

Signed-off-by: Dan McGee <dan at archlinux.org>
---
 lib/libalpm/dload.c |   24 ++++++++++++++----------
 1 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 83060f9..cd2857c 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -127,13 +127,14 @@ static int curl_progress(void *file, double dltotal, double dlnow,
 	return 0;
 }
 
-static int curl_gethost(const char *url, char *buffer)
+static int curl_gethost(const char *url, char *buffer, size_t buf_len)
 {
 	size_t hostlen;
 	char *p, *q;
 
 	if(strncmp(url, "file://", 7) == 0) {
-		strcpy(buffer, _("disk"));
+		p = _("disk");
+		hostlen = strlen(p);
 	} else {
 		p = strstr(url, "//");
 		if(!p) {
@@ -154,13 +155,14 @@ static int curl_gethost(const char *url, char *buffer)
 			hostlen -= q - p + 1;
 			p = q + 1;
 		}
+	}
 
-		if(hostlen > 255) {
-			/* buffer overflow imminent */
-			return 1;
-		}
-		snprintf(buffer, hostlen + 1, "%s", p);
+	if(hostlen > buf_len - 1) {
+		/* buffer overflow imminent */
+		return 1;
 	}
+	memcpy(buffer, p, hostlen);
+	buffer[hostlen] = '\0';
 
 	return 0;
 }
@@ -310,14 +312,16 @@ static FILE *create_tempfile(struct dload_payload *payload, const char *localpat
 	return fp;
 }
 
+/* RFC1123 states applications should support this length */
+#define HOSTNAME_SIZE 256
+
 static int curl_download_internal(struct dload_payload *payload,
 		const char *localpath, char **final_file)
 {
 	int ret = -1;
 	FILE *localf = NULL;
 	char *effective_url;
-	/* RFC1123 states applications should support this length */
-	char hostname[256];
+	char hostname[HOSTNAME_SIZE];
 	char error_buffer[CURL_ERROR_SIZE] = {0};
 	struct stat st;
 	long timecond, respcode = 0, remote_time = -1;
@@ -332,7 +336,7 @@ static int curl_download_internal(struct dload_payload *payload,
 	if(!payload->remote_name) {
 		payload->remote_name = strdup(get_filename(payload->fileurl));
 	}
-	if(!payload->remote_name || curl_gethost(payload->fileurl, hostname) != 0) {
+	if(!payload->remote_name || curl_gethost(payload->fileurl, hostname, sizeof(hostname)) != 0) {
 		_alpm_log(handle, ALPM_LOG_ERROR, _("url '%s' is invalid\n"), payload->fileurl);
 		RET_ERR(handle, ALPM_ERR_SERVER_BAD_URL, -1);
 	}
-- 
1.7.7