In case you didn't see this.... did we fix these or just accept the CERN
patch and set MAX=HUGE? Paul posted a little bit later showing another hole.
Anyone want to do a rewrite using dynamically allocated strings instead
of stack variables?
Brian
---------- Forwarded message ----------
Date: Tue, 11 Apr 1995 20:59:05 -0700
From: Paul Phillips <paulp@cerf.net>
Cc: bugtraq@fc.net, www-security@ns1.rutgers.edu
Newgroups: comp.infosystems.www.providers, comp.security.unix
Subject: Still security holes in NCSA httpd 1.3R
NCSA has been claiming at <URL:http://hoohoo.ncsa.uiuc.edu> that their
patch to httpd, which fixed the hole in strsubfirst, was sufficient
to fix the stack overflow bug discovered by Thomas Lopatic. This is
incorrect. There is at least one more instance of the same problem,
this one in log_reason. NCSA has provided a performance reason not to
#define MAX_STRING_LEN to HUGE_STRING_LEN (i.e. you will enter swap
hell) and they are correct about that, but unfortunately that doesn't
magically make the hole go away.
>>> begin code traversal >>>
When accept is handed some data, httpd.c calls process_request in
http_request.c:
213: process_request(0,stdout);
process_request allocates url[HUGE_STRING_LEN] for parsing out
the URL. It calls getline in util.c to obtain the info from the
socket:
104: if(getline(l,HUGE_STRING_LEN,in,timeout))
process_request now decides what the method is, and calls the
relevant function. Take GET for example: it calls process_get
in http_get.c with the url as an argument:
143: process_get(in,out,m,url,args);
Let's say it's a standard document, so process_get calls send_node
with the url as an argument (still possibly HUGE_STRING_LEN long)
195: send_node(url,args,in,out);
Now send_node does various checks. Let's say the file does not
exist: send_node wants to log it, here:
122: log_reason("file does not exist",file);
where file is the passed url, still possible 8192 characters.
log_reason is in http_log.c, and quite short:
void log_reason(char *reason, char *file) {
char t[MAX_STRING_LEN];
sprintf(t,"httpd: access to %s failed for %s, reason: %s",
file,remote_name,reason);
log_error(t);
}
The file can be 8192 characters, but it's sprintfed into a 256
character buffer.
>>> end code traversal >>>
This particular traversal is just an example, log_reason is called
from several places with overlarge buffers.
I pointed this out to the httpd people at NCSA, who promised a
patch but haven't acted with much alacrity. Frankly I am very
skeptical that any patch will fix all holes until I finish checking
out all the code. When 1.4 is released to the public I will
examine it in depth as soon as possible.
Regards,
--
Paul Phillips
paulp@cerf.net
patch and set MAX=HUGE? Paul posted a little bit later showing another hole.
Anyone want to do a rewrite using dynamically allocated strings instead
of stack variables?
Brian
---------- Forwarded message ----------
Date: Tue, 11 Apr 1995 20:59:05 -0700
From: Paul Phillips <paulp@cerf.net>
Cc: bugtraq@fc.net, www-security@ns1.rutgers.edu
Newgroups: comp.infosystems.www.providers, comp.security.unix
Subject: Still security holes in NCSA httpd 1.3R
NCSA has been claiming at <URL:http://hoohoo.ncsa.uiuc.edu> that their
patch to httpd, which fixed the hole in strsubfirst, was sufficient
to fix the stack overflow bug discovered by Thomas Lopatic. This is
incorrect. There is at least one more instance of the same problem,
this one in log_reason. NCSA has provided a performance reason not to
#define MAX_STRING_LEN to HUGE_STRING_LEN (i.e. you will enter swap
hell) and they are correct about that, but unfortunately that doesn't
magically make the hole go away.
>>> begin code traversal >>>
When accept is handed some data, httpd.c calls process_request in
http_request.c:
213: process_request(0,stdout);
process_request allocates url[HUGE_STRING_LEN] for parsing out
the URL. It calls getline in util.c to obtain the info from the
socket:
104: if(getline(l,HUGE_STRING_LEN,in,timeout))
process_request now decides what the method is, and calls the
relevant function. Take GET for example: it calls process_get
in http_get.c with the url as an argument:
143: process_get(in,out,m,url,args);
Let's say it's a standard document, so process_get calls send_node
with the url as an argument (still possibly HUGE_STRING_LEN long)
195: send_node(url,args,in,out);
Now send_node does various checks. Let's say the file does not
exist: send_node wants to log it, here:
122: log_reason("file does not exist",file);
where file is the passed url, still possible 8192 characters.
log_reason is in http_log.c, and quite short:
void log_reason(char *reason, char *file) {
char t[MAX_STRING_LEN];
sprintf(t,"httpd: access to %s failed for %s, reason: %s",
file,remote_name,reason);
log_error(t);
}
The file can be 8192 characters, but it's sprintfed into a 256
character buffer.
>>> end code traversal >>>
This particular traversal is just an example, log_reason is called
from several places with overlarge buffers.
I pointed this out to the httpd people at NCSA, who promised a
patch but haven't acted with much alacrity. Frankly I am very
skeptical that any patch will fix all holes until I finish checking
out all the code. When 1.4 is released to the public I will
examine it in depth as soon as possible.
Regards,
--
Paul Phillips
paulp@cerf.net