httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From d...@ast.cam.ac.uk (David Robinson)
Subject Re: Possible security problem in referer_log_transaction... ? (fwd)
Date Tue, 19 Dec 1995 12:23:00 GMT
Yes, there are potential security holes here.
The following patch fixes all the ones that I can find.

Many of the uses of MAX_STRING_LEN were in code like this (from http_main.c):

	    char errstr[MAX_STRING_LEN];
	    sprintf (errstr, "Memory hog alert: allocated %ld bytes for %s",
	             bytes_in_pool (ptrans), r->the_request);
            log_error (errstr, r->server);

so I've created a log_printf routine (with the code copied from rprintf())
which removes all these cases; there were many of them in mod_include.c

This left mod_cookies.c, mod_log_common.c (!) and mod_log_referer.c which I
fixed using pstrcat(). In the process I deleted a couple of occurrences of
the following horrible piece of code:

        sprintf(str,"%s%d ",str,r->status);

I've uploaded the patch to the for_Apache_1.0.0 directory.

 David.

P.S. I renamed 69.preserve_redirect.patch to 61a.preserve_redirect.patch

------- begin file 69.security.patch
From: drtr@ast.cam.ac.uk (David Robinson)
Subject: Fix possible security holes due to stack overwriting
Affects: http_log.c, http_log.h, http_main.c, mod_cookies.c, mod_include.c,
         mod_log_common.c, mod_log_referer.c
ChangeLog: Remove uses of MAX_STRING_LEN/HUGE_STRING_LEN from several routines.
Comments: This patch was made against the 1.0.1 distribution

*** http_log.c.orig	Sun Dec 17 14:32:29 1995
--- http_log.c	Tue Dec 19 11:22:17 1995
***************
*** 64,69 ****
--- 64,71 ----
  #include "http_config.h"
  #include "http_log.h"
  
+ #include <stdarg.h>
+ 
  void open_error_log(server_rec *s, pool *p)
  {
      char *fname;
***************
*** 119,124 ****
--- 121,139 ----
  void log_error(char *err, server_rec *s) {
      fprintf(s->error_log, "[%s] %s\n",get_time(),err);
      fflush(s->error_log);
+ }
+ 
+ void
+ log_printf(const server_rec *s, const char *fmt, ...)
+ {
+     va_list args;
+     
+     fprintf(s->error_log, "[%s] ", get_time());
+     va_start (args, fmt);
+     vfprintf (s->error_log, fmt, args);
+     va_end (args);
+ 
+     fputc('\n', s->error_log);
  }
  
  void log_reason(char *reason, char *file, request_rec *r) {
*** http_log.h.orig	Sun Dec 17 14:32:28 1995
--- http_log.h	Tue Dec 19 10:46:56 1995
***************
*** 57,61 ****
--- 57,62 ----
  
  void log_pid (pool *p, char *pid_fname);
  void log_error(char *err, server_rec *s);
+ void log_printf(const server_rec *s, const char *fmt, ...);
  void log_reason(char *reason, char *fname, request_rec *r);
  
*** http_main.c.orig	Sun Dec 17 14:32:29 1995
--- http_main.c	Tue Dec 19 10:47:48 1995
***************
*** 815,826 ****
  	r = read_request (current_conn);
  	if (r) process_request (r); /* else premature EOF --- ignore */
  		
! 	if (bytes_in_pool (ptrans) > 80000) {
! 	    char errstr[MAX_STRING_LEN];
! 	    sprintf (errstr, "Memory hog alert: allocated %ld bytes for %s",
! 	             bytes_in_pool (ptrans), r->the_request);
!             log_error (errstr, r->server);
!         }
  		
  	fflush (conn_out);
  	pfclose (ptrans,conn_in);
--- 815,824 ----
  	r = read_request (current_conn);
  	if (r) process_request (r); /* else premature EOF --- ignore */
  		
! 	if (bytes_in_pool (ptrans) > 80000)
! 	    log_printf(r->server,
! 		       "Memory hog alert: allocated %ld bytes for %s",
! 		       bytes_in_pool (ptrans), r->the_request);
  		
  	fflush (conn_out);
  	pfclose (ptrans,conn_in);
*** mod_cookies.c.orig	Sun Dec 17 14:32:29 1995
--- mod_cookies.c	Tue Dec 19 10:37:16 1995
***************
*** 171,177 ****
  {
      cookie_log_state *cls = get_module_config (orig->server->module_config,
                             &cookies_module);
!     char str[HUGE_STRING_LEN];
      long timz;
      struct tm *t;
      char tstr[MAX_STRING_LEN],sign;
--- 171,177 ----
  {
      cookie_log_state *cls = get_module_config (orig->server->module_config,
                             &cookies_module);
!     char *str;
      long timz;
      struct tm *t;
      char tstr[MAX_STRING_LEN],sign;
***************
*** 192,212 ****
      if(timz < 0) 
          timz = -timz;
  
!     strftime(tstr,MAX_STRING_LEN,"%d/%b/%Y:%H:%M:%S",t);
! 
!     sprintf(str,"%s \"%s\" [%s %c%02ld%02ld] ",
! 	    cookie+2,		/* Ignore s= */
! 	    orig->the_request,
!             tstr,
!             sign,
!             timz/3600,
!             timz%3600);
!     
      if (r->status != -1)
!         sprintf(str,"%s%d\n",str,r->status);
!     else
!         strcat(str,"-\n");
  
      write(cls->log_fd, str, strlen(str));
  
      return OK;
--- 192,207 ----
      if(timz < 0) 
          timz = -timz;
  
!     strftime(tstr,MAX_STRING_LEN,"\" [%d/%b/%Y:%H:%M:%S ",t);
      if (r->status != -1)
! 	sprintf(&tstr[strlen(tstr)], "%c%02ld%02ld] %d\n", sign, timz/3600,
! 		timz%3600, r->status);
! 	sprintf(&tstr[strlen(tstr)], "%c%02ld%02ld] -\n", sign, timz/3600,
! 		timz%3600);
  
+ /* ignore s= on cookie */
+     str = pstrcat(orig->pool, cookie + 2, " \", orig->the_request, tstr);
+     
      write(cls->log_fd, str, strlen(str));
  
      return OK;
*** mod_include.c.orig	Sun Dec 17 14:32:29 1995
--- mod_include.c	Tue Dec 19 10:54:50 1995
***************
*** 353,359 ****
  }
  
  int handle_include(FILE *in, request_rec *r, char *error, int noexec) {
!     char tag[MAX_STRING_LEN],errstr[MAX_STRING_LEN];
      char *tag_val;
  
      while(1) {
--- 353,359 ----
  }
  
  int handle_include(FILE *in, request_rec *r, char *error, int noexec) {
!     char tag[MAX_STRING_LEN];
      char *tag_val;
  
      while(1) {
***************
*** 396,403 ****
  	        error_fmt = "unable to include %s in parsed file %s";
  		    
              if (error_fmt) {
!                 sprintf(errstr, error_fmt, tag_val, r->filename);
!                 log_error (errstr, r->server);
                  rprintf(r,"%s",error);
              }            
  
--- 396,402 ----
  	        error_fmt = "unable to include %s in parsed file %s";
  		    
              if (error_fmt) {
!                 log_printf(r->server, error_fmt, tag_val, r->filename);
                  rprintf(r,"%s",error);
              }            
  
***************
*** 406,414 ****
          else if(!strcmp(tag,"done"))
              return 0;
          else {
!             sprintf(errstr,"unknown parameter %s to tag include in %s",tag,
! 		    r->filename);
!             log_error (errstr, r->server);
              rprintf(r,"%s",error);
          }
      }
--- 405,412 ----
          else if(!strcmp(tag,"done"))
              return 0;
          else {
!             log_printf(r->server, "unknown parameter %s to tag include in %s",
! 		       tag, r->filename);
              rprintf(r,"%s",error);
          }
      }
***************
*** 495,501 ****
  
  int handle_exec(FILE *in, request_rec *r, char *error)
  {
!     char tag[MAX_STRING_LEN],errstr[MAX_STRING_LEN];
      char *tag_val;
      char *file = r->filename;
  
--- 493,499 ----
  
  int handle_exec(FILE *in, request_rec *r, char *error)
  {
!     char tag[MAX_STRING_LEN];
      char *tag_val;
      char *file = r->filename;
  
***************
*** 504,511 ****
              return 1;
          if(!strcmp(tag,"cmd")) {
              if(include_cmd(tag_val, r) == -1) {
!                 sprintf(errstr,"failed command exec %s in %s",tag_val,file);
!                 log_error (errstr, r->server);
                  rprintf(r,"%s",error);
              }
              /* just in case some stooge changed directories */
--- 502,509 ----
              return 1;
          if(!strcmp(tag,"cmd")) {
              if(include_cmd(tag_val, r) == -1) {
!                 log_printf(r->server, "failed command exec %s in %s",
! 			   tag_val, file);
                  rprintf(r,"%s",error);
              }
              /* just in case some stooge changed directories */
***************
*** 513,520 ****
          } 
          else if(!strcmp(tag,"cgi")) {
              if(include_cgi(tag_val, r) == -1) {
!                 sprintf(errstr,"invalid CGI ref %s in %s",tag_val,file);
!                 log_error (errstr, r->server);
                  rprintf(r,"%s",error);
              }
              /* grumble groan */
--- 511,517 ----
          } 
          else if(!strcmp(tag,"cgi")) {
              if(include_cgi(tag_val, r) == -1) {
!                 log_printf(r->server, "invalid CGI ref %s in %s",tag_val,file);
                  rprintf(r,"%s",error);
              }
              /* grumble groan */
***************
*** 523,531 ****
          else if(!strcmp(tag,"done"))
              return 0;
          else {
!             char errstr[MAX_STRING_LEN];
!             sprintf(errstr,"unknown parameter %s to tag exec in %s",tag,file);
!             log_error (errstr, r->server);
              rprintf(r, "%s",error);
          }
      }
--- 520,527 ----
          else if(!strcmp(tag,"done"))
              return 0;
          else {
!             log_printf(r->server, "unknown parameter %s to tag exec in %s",
! 		       tag, file);
              rprintf(r, "%s",error);
          }
      }
***************
*** 547,556 ****
          } else if(!strcmp(tag,"done"))
              return 0;
          else {
!             char errstr[MAX_STRING_LEN];
!             sprintf(errstr,"unknown parameter %s to tag echo in %s",
  		    tag, r->filename);
-             log_error(errstr, r->server);
              rprintf (r, "%s", error);
          }
      }
--- 543,550 ----
          } else if(!strcmp(tag,"done"))
              return 0;
          else {
!             log_printf(r->server, "unknown parameter %s to tag echo in %s",
  		    tag, r->filename);
              rprintf (r, "%s", error);
          }
      }
***************
*** 584,593 ****
          else if(!strcmp(tag,"done"))
              return 0;
          else {
!             char errstr[MAX_STRING_LEN];
!             sprintf(errstr,"unknown parameter %s to tag config in %s",
                      tag, r->filename);
-             log_error(errstr, r->server);
              rprintf (r,"%s",error);
          }
      }
--- 578,585 ----
          else if(!strcmp(tag,"done"))
              return 0;
          else {
!             log_printf(r->server, "unknown parameter %s to tag config in %s",
                      tag, r->filename);
              rprintf (r,"%s",error);
          }
      }
***************
*** 598,604 ****
  int find_file(request_rec *r, char *directive, char *tag, 
                char *tag_val, struct stat *finfo, char *error)
  {
!     char errstr[MAX_STRING_LEN], dir[MAX_STRING_LEN];
      char *to_send;
  
      if(!strcmp(tag,"file")) {
--- 590,596 ----
  int find_file(request_rec *r, char *directive, char *tag, 
                char *tag_val, struct stat *finfo, char *error)
  {
!     char dir[MAX_STRING_LEN];
      char *to_send;
  
      if(!strcmp(tag,"file")) {
***************
*** 606,615 ****
          getwd(dir);
          to_send = make_full_path (r->pool, dir, tag_val);
          if(stat(to_send,finfo) == -1) {
!             sprintf(errstr,
                      "unable to get information about %s in parsed file %s",
                      to_send, r->filename);
-             log_error (errstr, r->server);
              rprintf (r,"%s",error);
              return -1;
          }
--- 598,606 ----
          getwd(dir);
          to_send = make_full_path (r->pool, dir, tag_val);
          if(stat(to_send,finfo) == -1) {
!             log_printf(r->server,
                      "unable to get information about %s in parsed file %s",
                      to_send, r->filename);
              rprintf (r,"%s",error);
              return -1;
          }
***************
*** 623,632 ****
  	    destroy_sub_req (rr);
  	    return 0;
          } else {
!             sprintf(errstr,
                      "unable to get information about %s in parsed file %s",
                      tag_val, r->filename);
-             log_error(errstr, r->server);
              rprintf(r,"%s",error);
  	    destroy_sub_req (rr);
              return -1;
--- 614,622 ----
  	    destroy_sub_req (rr);
  	    return 0;
          } else {
!             log_printf(r->server,
                      "unable to get information about %s in parsed file %s",
                      tag_val, r->filename);
              rprintf(r,"%s",error);
  	    destroy_sub_req (rr);
              return -1;
***************
*** 633,641 ****
          }
      }
      else {
!         sprintf(errstr,"unknown parameter %s to tag %s in %s",
                  tag, directive, r->filename);
-         log_error(errstr, r->server);
          rprintf(r,"%s",error);
          return -1;
      }
--- 623,630 ----
          }
      }
      else {
!         log_printf(r->server, "unknown parameter %s to tag %s in %s",
                  tag, directive, r->filename);
          rprintf(r,"%s",error);
          return -1;
      }
***************
*** 701,707 ****
  void send_parsed_content(FILE *f, request_rec *r)
  {
      char directive[MAX_STRING_LEN], error[MAX_STRING_LEN];
!     char timefmt[MAX_STRING_LEN], errstr[MAX_STRING_LEN];
      int noexec = allow_options (r) & OPT_INCNOEXEC;
      int ret, sizefmt;
  
--- 690,696 ----
  void send_parsed_content(FILE *f, request_rec *r)
  {
      char directive[MAX_STRING_LEN], error[MAX_STRING_LEN];
!     char timefmt[MAX_STRING_LEN];
      int noexec = allow_options (r) & OPT_INCNOEXEC;
      int ret, sizefmt;
  
***************
*** 717,725 ****
                  return;
              if(!strcmp(directive,"exec")) {
                  if(noexec) {
!                     sprintf(errstr,"httpd: exec used but not allowed in %s",
!                             r->filename);
!                     log_error (errstr, r->server);
                      rprintf(r,"%s",error);
                      ret = find_string(f,ENDING_SEQUENCE,NULL);
                  } else 
--- 706,714 ----
                  return;
              if(!strcmp(directive,"exec")) {
                  if(noexec) {
!                     log_printf(r->server,
! 			       "httpd: exec used but not allowed in %s",
! 			       r->filename);
                      rprintf(r,"%s",error);
                      ret = find_string(f,ENDING_SEQUENCE,NULL);
                  } else 
***************
*** 736,751 ****
              else if(!strcmp(directive,"flastmod"))
                  ret=handle_flastmod(f, r, error, timefmt);
              else {
!                 sprintf(errstr,"httpd: unknown directive %s in parsed doc %s",
!                         directive,r->filename);
!                 log_error (errstr, r->server);
                  rprintf (r,"%s",error);
                  ret=find_string(f,ENDING_SEQUENCE,NULL);
              }
              if(ret) {
!                 sprintf(errstr,"httpd: premature EOF in parsed file %s",
! 			r->filename);
!                 log_error(errstr, r->server);
                  return;
              }
          } else 
--- 725,739 ----
              else if(!strcmp(directive,"flastmod"))
                  ret=handle_flastmod(f, r, error, timefmt);
              else {
!                 log_printf(r->server, 
! 			   "httpd: unknown directive %s in parsed doc %s",
! 			   directive, r->filename);
                  rprintf (r,"%s",error);
                  ret=find_string(f,ENDING_SEQUENCE,NULL);
              }
              if(ret) {
!                 log_printf(r->server, "httpd: premature EOF in parsed file %s",
! 			   r->filename);
                  return;
              }
          } else 
*** mod_log_common.c.orig	Sun Dec 17 14:32:29 1995
--- mod_log_common.c	Tue Dec 19 12:16:26 1995
***************
*** 145,154 ****
      common_log_state *cls = get_module_config (orig->server->module_config,
  					       &common_log_module);
    
!     char str[HUGE_STRING_LEN];
      long timz;
      struct tm *t;
!     char tstr[MAX_STRING_LEN],sign;
      conn_rec *c = orig->connection;
      request_rec *r;
  
--- 145,154 ----
      common_log_state *cls = get_module_config (orig->server->module_config,
  					       &common_log_module);
    
!     char *str;
      long timz;
      struct tm *t;
!     char tstr[MAX_STRING_LEN], status[MAX_STRING_LEN], sign;
      conn_rec *c = orig->connection;
      request_rec *r;
  
***************
*** 167,194 ****
      if(timz < 0) 
          timz = -timz;
  
!     strftime(tstr,MAX_STRING_LEN,"%d/%b/%Y:%H:%M:%S",t);
  
!     sprintf(str,"%s %s %s [%s %c%02ld%02ld] \"%s\" ",
!             c->remote_name,
!             (c->remote_logname ? c->remote_logname : "-"),
! 	    (c->user ? c->user : "-"),
!             tstr,
!             sign,
!             timz/3600,
!             timz%3600,
!             (orig->the_request ? orig->the_request : "NULL") );
!     
!     if (r->status != -1)
!         sprintf(str,"%s%d ",str,r->status);
!     else
!         strcat(str,"- ");
  
      if(r->bytes_sent != -1)
!         sprintf(str,"%s%d\n",str,r->bytes_sent);
      else
!         strcat(str,"-\n");
  
      write(cls->log_fd, str, strlen(str));
  
      return OK;
--- 167,192 ----
      if(timz < 0) 
          timz = -timz;
  
!     strftime(tstr,MAX_STRING_LEN," [%d/%b/%Y:%H:%M:%S",t);
!     sprintf(&tstr[strlen(tstr)], " %c%02ld%02ld] \"", sign, timz/3600,
! 	    timz%3600);
  
!     if (r->status != -1) sprintf(status,"%d ", r->status);
!     else                 strcpy(status, "- ");
  
      if(r->bytes_sent != -1)
! 	sprintf(&status[strlen(status)], "%d\n", r->bytes_sent);
      else
!         strcat(status, "-\n");
  
+     str = pstrcat(orig->pool, c->remote_name, " ",
+ 		  (c->remote_logname != NULL ? c->remote_logname : "-"),
+ 		  " ",
+ 		  (c->user != NULL ? c->user : "-"),
+ 		  tstr, 
+ 		  (orig->the_request != NULL ? orig->the_request : "NULL"), 
+ 		  "\" ", status, NULL);
+     
      write(cls->log_fd, str, strlen(str));
  
      return OK;
*** mod_log_referer.c.orig	Sun Dec 17 14:32:29 1995
--- mod_log_referer.c	Tue Dec 19 11:18:23 1995
***************
*** 162,168 ****
      referer_log_state *cls = get_module_config (orig->server->module_config,
  					       &referer_log_module);
    
!     char str[HUGE_STRING_LEN];
      char *referer;
      request_rec *r;
  
--- 162,168 ----
      referer_log_state *cls = get_module_config (orig->server->module_config,
  					       &referer_log_module);
    
!     char *str;
      char *referer;
      request_rec *r;
  
***************
*** 199,206 ****
  	    }
  	  
  	  
! 	  sprintf(str, "%s -> %s\n", referer,
! 		  r->uri);
  	  write(cls->referer_fd, str, strlen(str));
        }
  
--- 199,205 ----
  	    }
  	  
  	  
! 	  str = pstrcat(orig->pool, referer, " -> ", r->uri, "\n", NULL);
  	  write(cls->referer_fd, str, strlen(str));
        }
  
-------- end file 69.security.patch

Mime
View raw message