bryson/pacman
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

curl_gethost() potential bug fixups

Browse source 
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>
This commit is contained in:
Dan McGee 2011-10-13 12:53:56 -05:00
parent 1ebe5dc197
commit 86bc36412e
1 changed files with 15 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
lib/libalpm/dload.c
View file

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