On Thu, Oct 13, 2011 at 12:56:25PM -0500, Dan McGee wrote:
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>
Ack. I'm embarassed for having written this. d
--- 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