diff options
author | Dan McGee <dan@archlinux.org> | 2011-10-13 12:53:56 -0500 |
---|---|---|
committer | Dan McGee <dan@archlinux.org> | 2011-10-13 15:16:10 -0500 |
commit | 86bc36412e2619e0e05d61cf6216ef68814cf1cd (patch) | |
tree | 134edcc935d085df5374b3b797d983628b6d6751 | |
parent | 1ebe5dc1979e90c37d6534d6b1e0173a884326b1 (diff) | |
download | pacman-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.c | 24 |
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); } |