summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDan McGee <dan@archlinux.org>2011-10-13 12:53:56 -0500
committerDan McGee <dan@archlinux.org>2011-10-13 15:16:10 -0500
commit86bc36412e2619e0e05d61cf6216ef68814cf1cd (patch)
tree134edcc935d085df5374b3b797d983628b6d6751
parent1ebe5dc1979e90c37d6534d6b1e0173a884326b1 (diff)
downloadpacman-86bc36412e2619e0e05d61cf6216ef68814cf1cd.tar.xz
curl_gethost() potential bug fixups
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@archlinux.org>
-rw-r--r--lib/libalpm/dload.c24
1 files changed, 14 insertions, 10 deletions
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 83060f97..cd2857c3 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);
}