You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Rob Hartill <ro...@imdb.com> on 1997/01/29 00:52:53 UTC
Re: fix for log file security problem documented in 1.2b6
Hi,
I'll forward your suggestion to the developers to see if there's
enough/any interest in adding it.
cheers,
rob.
On Tue, 28 Jan 1997, David J MacKenzie wrote:
> From: David MacKenzie <dj...@va.pubnix.com>
> Subject: fix for log file security problem documented in 1.2b6
> Affects: http_log.c http_log.h mod_log_agent.c mod_log_config.c mod_log_referer.c
> ChangeLog: New function to check whether a log file is safe to open.
> Comments: Addresses the security hole newly documented in 1.2b6, if
> I've understood it correctly.
> We need this because we put our users' error log files under their
> home directory, so it counts against their disk quota.
>
> Index: http_log.c
> ===================================================================
> RCS file: /export/src/CVS/usr.local/libexec/apache_1.2b6/src/http_log.c,v
> retrieving revision 1.1
> diff -u -r1.1 http_log.c
> --- http_log.c 1997/01/28 20:22:52 1.1
> +++ http_log.c 1997/01/28 22:13:16
> @@ -65,6 +65,25 @@
>
> #include <stdarg.h>
>
> +void check_log_safety(const char *name)
> +{
> + struct stat sb;
> +
> + if (lstat(name, &sb) != 0)
> + return;
> + if (!S_ISREG(sb.st_mode)) {
> + fprintf(stderr,
> + "httpd: security hole: log file %s is not a regular file\n",
> + name);
> + exit(1);
> + }
> + if (sb.st_nlink != 1) {
> + fprintf(stderr, "httpd: security hole: log file %s has %d links\n",
> + name, sb.st_nlink);
> + exit(1);
> + }
> +}
> +
> void error_log_child (void *cmd)
> {
> /* Child process code for 'ErrorLog "|..."';
> @@ -102,6 +121,7 @@
>
> s->error_log = dummy;
> } else {
> + check_log_safety(fname);
> if(!(s->error_log = pfopen(p, fname, "a"))) {
> fprintf(stderr,"httpd: could not open error log file %s.\n", fname);
> perror("fopen");
> @@ -141,6 +161,7 @@
>
> if (!pid_fname) return;
> pid_fname = server_root_relative (p, pid_fname);
> + check_log_safety(pid_fname);
> if(!(pid_file = fopen(pid_fname,"w"))) {
> perror("fopen");
> fprintf(stderr,"httpd: could not log pid to file %s\n", pid_fname);
> Index: http_log.h
> ===================================================================
> RCS file: /export/src/CVS/usr.local/libexec/apache_1.2b6/src/http_log.h,v
> retrieving revision 1.1
> diff -u -r1.1 http_log.h
> --- http_log.h 1997/01/28 20:22:53 1.1
> +++ http_log.h 1997/01/28 22:13:16
> @@ -53,6 +53,7 @@
> void open_logs (server_rec *, pool *p);
> void error_log2stderr (server_rec *);
>
> +void check_log_safety(const char *name);
> void log_pid (pool *p, char *pid_fname);
> void log_error(char *err, server_rec *s);
> extern void log_unixerr(const char *routine, const char *file,
> Index: mod_log_agent.c
> ===================================================================
> RCS file: /export/src/CVS/usr.local/libexec/apache_1.2b6/src/mod_log_agent.c,v
> retrieving revision 1.1
> diff -u -r1.1 mod_log_agent.c
> --- mod_log_agent.c 1997/01/28 20:22:58 1.1
> +++ mod_log_agent.c 1997/01/28 22:13:16
> @@ -137,6 +137,7 @@
> cls->agent_fd = fileno (dummy);
> }
> else if(*cls->fname != '\0') {
> + check_log_safety(fname);
> if((cls->agent_fd = popenf(p, fname, xfer_flags, xfer_mode)) < 0) {
> fprintf(stderr,"httpd: could not open agent log file %s.\n", fname);
> perror("open");
> Index: mod_log_config.c
> ===================================================================
> RCS file: /export/src/CVS/usr.local/libexec/apache_1.2b6/src/mod_log_config.c,v
> retrieving revision 1.2
> diff -u -r1.2 mod_log_config.c
> --- mod_log_config.c 1997/01/28 20:41:34 1.2
> +++ mod_log_config.c 1997/01/28 22:13:16
> @@ -722,6 +722,7 @@
> }
> else {
> char *fname = server_root_relative (p, cls->fname);
> + check_log_safety(fname);
> if((cls->log_fd = popenf(p, fname, xfer_flags, xfer_mode)) < 0) {
> fprintf (stderr,
> "httpd: could not open transfer log file %s.\n", fname);
> Index: mod_log_referer.c
> ===================================================================
> RCS file: /export/src/CVS/usr.local/libexec/apache_1.2b6/src/mod_log_referer.c,v
> retrieving revision 1.1
> diff -u -r1.1 mod_log_referer.c
> --- mod_log_referer.c 1997/01/28 20:22:58 1.1
> +++ mod_log_referer.c 1997/01/28 22:13:16
> @@ -152,6 +152,7 @@
> cls->referer_fd = fileno (dummy);
> }
> else if(*cls->fname != '\0') {
> + check_log_safety(fname);
> if((cls->referer_fd = popenf(p, fname, xfer_flags, xfer_mode)) < 0) {
> fprintf(stderr,"httpd: could not open referer log file %s.\n", fname);
> perror("open");
>
>
>
_______________________________________________________________________
Rob Hartill. Internet Movie Database Ltd. http://www.imdb.com/
Re: fix for log file security problem documented in 1.2b6
Posted by Marc Slemko <ma...@znep.com>.
The problem with this is that it is not secure; there is a race condition
in between where you check the file and when it is opened. If the checks
are seperate, they need to be atomic so no one can switch the files after
the checks but before the open. This solution just makes it a little
harder to exploit. With a fstat() you can get around that because you
have a filehandle. If there was a flstat() you could do this, but there
isn't.
I am still contemplating the value of checks like this to make it more
difficult to exploit, but they do _not_ solve the problem. The race
condition may appear to be minute at first look, but there are very often
ways to force the server to block on something so there is time to switch
the file.
On Tue, 28 Jan 1997, Rob Hartill wrote:
>
> Hi,
>
> I'll forward your suggestion to the developers to see if there's
> enough/any interest in adding it.
>
> cheers,
> rob.
>
> On Tue, 28 Jan 1997, David J MacKenzie wrote:
>
> > From: David MacKenzie <dj...@va.pubnix.com>
> > Subject: fix for log file security problem documented in 1.2b6
> > Affects: http_log.c http_log.h mod_log_agent.c mod_log_config.c mod_log_referer.c
> > ChangeLog: New function to check whether a log file is safe to open.
> > Comments: Addresses the security hole newly documented in 1.2b6, if
> > I've understood it correctly.
> > We need this because we put our users' error log files under their
> > home directory, so it counts against their disk quota.
> >
> > Index: http_log.c
> > ===================================================================
> > RCS file: /export/src/CVS/usr.local/libexec/apache_1.2b6/src/http_log.c,v
> > retrieving revision 1.1
> > diff -u -r1.1 http_log.c
> > --- http_log.c 1997/01/28 20:22:52 1.1
> > +++ http_log.c 1997/01/28 22:13:16
> > @@ -65,6 +65,25 @@
> >
> > #include <stdarg.h>
> >
> > +void check_log_safety(const char *name)
> > +{
> > + struct stat sb;
> > +
> > + if (lstat(name, &sb) != 0)
> > + return;
> > + if (!S_ISREG(sb.st_mode)) {
> > + fprintf(stderr,
> > + "httpd: security hole: log file %s is not a regular file\n",
> > + name);
> > + exit(1);
> > + }
> > + if (sb.st_nlink != 1) {
> > + fprintf(stderr, "httpd: security hole: log file %s has %d links\n",
> > + name, sb.st_nlink);
> > + exit(1);
> > + }
> > +}
> > +
> > void error_log_child (void *cmd)
> > {
> > /* Child process code for 'ErrorLog "|..."';
> > @@ -102,6 +121,7 @@
> >
> > s->error_log = dummy;
> > } else {
> > + check_log_safety(fname);
> > if(!(s->error_log = pfopen(p, fname, "a"))) {
> > fprintf(stderr,"httpd: could not open error log file %s.\n", fname);
> > perror("fopen");
> > @@ -141,6 +161,7 @@
> >
> > if (!pid_fname) return;
> > pid_fname = server_root_relative (p, pid_fname);
> > + check_log_safety(pid_fname);
> > if(!(pid_file = fopen(pid_fname,"w"))) {
> > perror("fopen");
> > fprintf(stderr,"httpd: could not log pid to file %s\n", pid_fname);
> > Index: http_log.h
> > ===================================================================
> > RCS file: /export/src/CVS/usr.local/libexec/apache_1.2b6/src/http_log.h,v
> > retrieving revision 1.1
> > diff -u -r1.1 http_log.h
> > --- http_log.h 1997/01/28 20:22:53 1.1
> > +++ http_log.h 1997/01/28 22:13:16
> > @@ -53,6 +53,7 @@
> > void open_logs (server_rec *, pool *p);
> > void error_log2stderr (server_rec *);
> >
> > +void check_log_safety(const char *name);
> > void log_pid (pool *p, char *pid_fname);
> > void log_error(char *err, server_rec *s);
> > extern void log_unixerr(const char *routine, const char *file,
> > Index: mod_log_agent.c
> > ===================================================================
> > RCS file: /export/src/CVS/usr.local/libexec/apache_1.2b6/src/mod_log_agent.c,v
> > retrieving revision 1.1
> > diff -u -r1.1 mod_log_agent.c
> > --- mod_log_agent.c 1997/01/28 20:22:58 1.1
> > +++ mod_log_agent.c 1997/01/28 22:13:16
> > @@ -137,6 +137,7 @@
> > cls->agent_fd = fileno (dummy);
> > }
> > else if(*cls->fname != '\0') {
> > + check_log_safety(fname);
> > if((cls->agent_fd = popenf(p, fname, xfer_flags, xfer_mode)) < 0) {
> > fprintf(stderr,"httpd: could not open agent log file %s.\n", fname);
> > perror("open");
> > Index: mod_log_config.c
> > ===================================================================
> > RCS file: /export/src/CVS/usr.local/libexec/apache_1.2b6/src/mod_log_config.c,v
> > retrieving revision 1.2
> > diff -u -r1.2 mod_log_config.c
> > --- mod_log_config.c 1997/01/28 20:41:34 1.2
> > +++ mod_log_config.c 1997/01/28 22:13:16
> > @@ -722,6 +722,7 @@
> > }
> > else {
> > char *fname = server_root_relative (p, cls->fname);
> > + check_log_safety(fname);
> > if((cls->log_fd = popenf(p, fname, xfer_flags, xfer_mode)) < 0) {
> > fprintf (stderr,
> > "httpd: could not open transfer log file %s.\n", fname);
> > Index: mod_log_referer.c
> > ===================================================================
> > RCS file: /export/src/CVS/usr.local/libexec/apache_1.2b6/src/mod_log_referer.c,v
> > retrieving revision 1.1
> > diff -u -r1.1 mod_log_referer.c
> > --- mod_log_referer.c 1997/01/28 20:22:58 1.1
> > +++ mod_log_referer.c 1997/01/28 22:13:16
> > @@ -152,6 +152,7 @@
> > cls->referer_fd = fileno (dummy);
> > }
> > else if(*cls->fname != '\0') {
> > + check_log_safety(fname);
> > if((cls->referer_fd = popenf(p, fname, xfer_flags, xfer_mode)) < 0) {
> > fprintf(stderr,"httpd: could not open referer log file %s.\n", fname);
> > perror("open");
> >
> >
> >
>
> _______________________________________________________________________
> Rob Hartill. Internet Movie Database Ltd. http://www.imdb.com/
>