* Moritz Muehlenhoff [2006-01-27 15:28:00+0100] > Recai Oktaş wrote: > > + Backport r1636 from upstream's Subversion repository: > > "Added IP address to log file" > > Why is r1636 necessary? This seems like a new feature (better logging > in case of an attack), but doesn't seem to fix a direct security problem > and could potentially break scripts that monitor the log file and expect > the current logfile file format.
I'll remove it. > The rest of the patch looks fine. Hmm, just found some other issues regarding this CVE-2005-4439. Previous tests had seemed fine to me, but when I made more tests, the bug came up again. I believe the attached patch should fix this completely. Stefan, could you have a look at it please? -- roktas
Subject: [PATCH]: More Fixes for CVE-2005-4439: buffer overflow through long URL parameters --- a/src/elogd.c 2006-01-27 10:27:21.000000000 +0200 +++ b/src/elogd.c 2006-01-28 01:31:33.000000000 +0200 @@ -23205,7 +23205,7 @@ void server_loop(void) { int status, i, n, n_error, authorized, min, i_min, i_conn, length; struct sockaddr_in serv_addr, acc_addr; - char pwd[256], str[1000], url[256], cl_pwd[256], *p, *pd; + char pwd[256], str[1000], url[256], cl_pwd[256], *p; char cookie[256], boundary[256], list[1000], theme[256], host_list[MAX_N_LIST][NAME_LENGTH], logbook[256], logbook_enc[256], global_cmd[256]; int lsock, len, flag, content_length, header_length; @@ -23756,7 +23756,7 @@ void server_loop(void) p = strchr(net_buffer, '/') + 1; /* check for ../.. to avoid serving of files on top of the elog directory */ - for (i = 0; p[i] && p[i] != ' ' && p[i] != '?'; i++) + for (i = 0; p[i] && p[i] != ' ' && p[i] != '?' && i < (int) sizeof(url); i++) url[i] = p[i]; url[i] = 0; @@ -23774,7 +23774,7 @@ void server_loop(void) } /* check if file is in scripts directory or in its subdirs */ - for (i = 0; p[i] && p[i] != ' ' && p[i] != '?'; i++) + for (i = 0; p[i] && p[i] != ' ' && p[i] != '?' && i < (int) sizeof(url); i++) url[i] = (p[i] == '/') ? DIR_SEPARATOR : p[i]; url[i] = 0; if (strchr(url, '.')) { @@ -23810,7 +23810,7 @@ void server_loop(void) } logbook[0] = 0; - for (i = 0; *p && *p != '/' && *p != '?' && *p != ' '; i++) + for (i = 0; *p && *p != '/' && *p != '?' && *p != ' ' && i < (int) sizeof(logbook); i++) logbook[i] = *p++; logbook[i] = 0; strcpy(logbook_enc, logbook); @@ -23831,10 +23831,9 @@ void server_loop(void) /* check for trailing '/' after logbook/ID */ if (logbook[0] && *p == '/' && *(p + 1) != ' ') { sprintf(url, "%s", logbook_enc); - pd = url + strlen(url); - while (*p && *p != ' ') - *pd++ = *p++; - *pd = 0; + for (i = strlen(url); *p && *p != ' ' && i < (int) sizeof(url); i++) + url[i] = *p++; + url[i] = 0; if (*(p - 1) == '/') { sprintf(str, "Invalid URL: %s", url); show_error(str); @@ -24109,7 +24108,8 @@ void server_loop(void) goto redir; } else if (strncmp(net_buffer, "GET", 3) == 0) { /* extract path and commands */ - *strchr(net_buffer, '\r') = 0; + if (strchr(net_buffer, '\r')) + *strchr(net_buffer, '\r') = 0; if (!strstr(net_buffer, "HTTP/1")) goto finished; *(strstr(net_buffer, "HTTP/1") - 1) = 0;
signature.asc
Description: Digital signature