You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by dg...@hyperreal.org on 1998/01/03 00:44:46 UTC

cvs commit: apachen/src/modules/standard mod_info.c

dgaudet     98/01/02 15:44:46

  Modified:    .        STATUS
               src      CHANGES
               src/modules/standard mod_info.c
  Log:
  - make mod_info_html_cmd_string() thread safe
  
  - fix minor buffer overrun in mod_info_html_cmd_string() (it would only
  hammer a \0 up to 5 bytes past the end of the buffer... nothing big)
  
  - mod_info_load_config() switched to use getword_conf() just like the real
  config parsing routines
  
  - replace a bunch of ap_snprintf()/rputs() pairs with rprintf() for more
  efficiency
  
  Reviewed by:	Brian Behlendorf
  
  Revision  Changes    Path
  1.45      +1 -4      apachen/STATUS
  
  Index: STATUS
  ===================================================================
  RCS file: /export/home/cvs/apachen/STATUS,v
  retrieving revision 1.44
  retrieving revision 1.45
  diff -u -r1.44 -r1.45
  --- STATUS	1998/01/02 17:03:16	1.44
  +++ STATUS	1998/01/02 23:44:42	1.45
  @@ -63,6 +63,7 @@
       * Jim's [PATCH] ap_cpystrn() function (replace strncpy) Take II
       * Dean's [PATCH] 1.3: "DoS" attack
       * Paul/Ben's [PATCH] 1.3: spaces in NT spawn* arguments
  +    * Dean's [PATCH] mod_info minor cleanups (take 2)
   
   Available Patches:
   
  @@ -79,10 +80,6 @@
       * Dean's [PATCH] mod_status cleanups
   	<Pi...@twinlark.arctic.org>
   	Status: Dean +1, Jim +1
  -
  -    * Dean's [PATCH] mod_info minor cleanups (take 2)
  -	<Pi...@twinlark.arctic.org>
  -	Status: Dean +1, Brian +1
   
       * Martin's [PATCH] 36kB: Make apache compile & run on an EBCDIC mainframe
   	<19...@deejai.mch.sni.de>
  
  
  
  1.554     +3 -0      apachen/src/CHANGES
  
  Index: CHANGES
  ===================================================================
  RCS file: /export/home/cvs/apachen/src/CHANGES,v
  retrieving revision 1.553
  retrieving revision 1.554
  diff -u -r1.553 -r1.554
  --- CHANGES	1997/12/30 19:03:16	1.553
  +++ CHANGES	1998/01/02 23:44:43	1.554
  @@ -1,5 +1,8 @@
   Changes with Apache 1.3b4
   
  +  *) A few cleanups in mod_info to make it thread-safe, and remove an
  +     off-by-5 bug that could hammer \0 on the stack. [Dean Gaudet]
  +
     *) no2slash() was O(n^2) in the length of the input.  Make it O(n).
        [Dean Gaudet]
   
  
  
  
  1.32      +66 -105   apachen/src/modules/standard/mod_info.c
  
  Index: mod_info.c
  ===================================================================
  RCS file: /export/home/cvs/apachen/src/modules/standard/mod_info.c,v
  retrieving revision 1.31
  retrieving revision 1.32
  diff -u -r1.31 -r1.32
  --- mod_info.c	1997/10/26 20:20:05	1.31
  +++ mod_info.c	1998/01/02 23:44:45	1.32
  @@ -119,27 +119,27 @@
       return new;
   }
   
  -static char *mod_info_html_cmd_string(char *string)
  +static char *mod_info_html_cmd_string(const char *string, char *buf, size_t buf_len)
   {
  -    char *s, *t;
  -    static char ret[256];       /* What is the max size of a command? */
  -    char *end_ret;
  +    const char *s;
  +    char *t;
  +    char *end_buf;
   
  -    ret[0] = '\0';
       s = string;
  -    t = ret;
  -    end_ret = t + sizeof(ret);
  -    while ((*s) && ((t - ret) < sizeof(ret))) {
  +    t = buf;
  +    /* keep space for \0 byte */
  +    end_buf = buf + buf_len - 1;
  +    while ((*s) && (t < end_buf)) {
           if (*s == '<') {
  -            strncpy(t, "&lt;", end_ret - t);
  +            strncpy(t, "&lt;", end_buf - t);
               t += 4;
           }
           else if (*s == '>') {
  -            strncpy(t, "&gt;", end_ret - t);
  +            strncpy(t, "&gt;", end_buf - t);
               t += 4;
           }
           else if (*s == '&') {
  -            strncpy(t, "&amp;", end_ret - t);
  +            strncpy(t, "&amp;", end_buf - t);
               t += 5;
           }
           else {
  @@ -147,25 +147,33 @@
           }
           s++;
       }
  -    *t = '\0';
  -    return (ret);
  +    /* oops, overflowed... don't overwrite */
  +    if (t > end_buf) {
  +	*end_buf = '\0';
  +    }
  +    else {
  +	*t = '\0';
  +    }
  +    return (buf);
   }
   
  -static info_cfg_lines *mod_info_load_config(pool *p, char *filename,
  +static info_cfg_lines *mod_info_load_config(pool *p, const char *filename,
                                               request_rec *r)
   {
       char s[MAX_STRING_LEN];
       configfile_t *fp;
  -    info_cfg_lines *new, *ret = NULL, *prev = NULL;
  -    char *t, *tt, o, *msg;
  +    info_cfg_lines *new, *ret, *prev;
  +    const char *t;
   
       fp = pcfg_openfile(p, filename);
       if (!fp) {
  -        msg = pstrcat(r->pool, "mod_info: couldn't open config file ",
  -                      filename, NULL);
  -        aplog_error(APLOG_MARK, APLOG_WARNING, r->server, msg);
  +        aplog_error(APLOG_MARK, APLOG_WARNING, r->server, 
  +		    "mod_info: couldn't open config file %s",
  +		    filename);
           return NULL;
       }
  +    ret = NULL;
  +    prev = NULL;
       while (!cfg_getline(s, MAX_STRING_LEN, fp)) {
           if (*s == '#') {
               continue;           /* skip comments */
  @@ -178,25 +186,14 @@
           if (prev) {
               prev->next = new;
           }
  -        t = strchr(s, ' ');
  -        tt = strchr(s, '\t');
  -        if (t && tt) {
  -            t = (t < tt) ? t : tt;
  -        }
  -        else if (tt) {
  -            t = tt;
  -        }
  -        if (t) {
  -            o = *t;
  -            *t = '\0';
  -            new->cmd = pstrdup(p, s);
  -            new->line = pstrdup(p, t + 1);
  -            *t = o;
  -        }
  -        else {
  -            new->cmd = pstrdup(p, s);
  -            new->line = NULL;
  -        }
  +	t = s;
  +	new->cmd = getword_conf(p, &t);
  +	if (*t) {
  +	    new->line = pstrdup(p, t);
  +	}
  +	else {
  +	    new->line = NULL;
  +	}
           prev = new;
       }
       cfg_closefile(fp);
  @@ -210,6 +207,7 @@
       info_cfg_lines *li = cfg, *li_st = NULL, *li_se = NULL;
       info_cfg_lines *block_start = NULL;
       int lab = 0, nest = 0;
  +    char buf[MAX_STRING_LEN];
   
       while (li) {
           if (!strncasecmp(li->cmd, "<directory", 10) ||
  @@ -237,10 +235,10 @@
                       if (nest == 2) {
                           rputs("&nbsp;&nbsp;", r);
                       }
  -                    rputs(mod_info_html_cmd_string(li->cmd), r);
  +                    rputs(mod_info_html_cmd_string(li->cmd, buf, sizeof(buf)), r);
                       rputs(" ", r);
                       if (li->line) {
  -                        rputs(mod_info_html_cmd_string(li->line), r);
  +                        rputs(mod_info_html_cmd_string(li->line, buf, sizeof(buf)), r);
                       }
                       rputs("</tt>\n", r);
                       nest--;
  @@ -291,19 +289,19 @@
                            strncasecmp(li->cmd, "</directory", 11) &&
                            strncasecmp(li->cmd, "</files", 7))) {
                           rputs("<dd><tt>", r);
  -                        rputs(mod_info_html_cmd_string(li_st->cmd), r);
  +                        rputs(mod_info_html_cmd_string(li_st->cmd, buf, sizeof(buf)), r);
                           rputs(" ", r);
                           if (li_st->line) {
  -                            rputs(mod_info_html_cmd_string(li_st->line), r);
  +                            rputs(mod_info_html_cmd_string(li_st->line, buf, sizeof(buf)), r);
                           }
                           rputs("</tt>\n", r);
                           block_start = li_st;
                           if (li_se) {
                               rputs("<dd><tt>&nbsp;&nbsp;", r);
  -                            rputs(mod_info_html_cmd_string(li_se->cmd), r);
  +                            rputs(mod_info_html_cmd_string(li_se->cmd, buf, sizeof(buf)), r);
                               rputs(" ", r);
                               if (li_se->line) {
  -                                rputs(mod_info_html_cmd_string(li_se->line), r);
  +                                rputs(mod_info_html_cmd_string(li_se->line, buf, sizeof(buf)), r);
                               }
                               rputs("</tt>\n", r);
                               block_start = li_se;
  @@ -316,10 +314,10 @@
                       if (nest == 2) {
                           rputs("&nbsp;&nbsp;", r);
                       }
  -                    rputs(mod_info_html_cmd_string(li->cmd), r);
  +                    rputs(mod_info_html_cmd_string(li->cmd, buf, sizeof(buf)), r);
                       if (li->line) {
                           rputs(" <i>", r);
  -                        rputs(mod_info_html_cmd_string(li->line), r);
  +                        rputs(mod_info_html_cmd_string(li->line, buf, sizeof(buf)), r);
                           rputs("</i></tt>", r);
                       }
                   }
  @@ -354,7 +352,7 @@
   static int display_info(request_rec *r)
   {
       module *modp = NULL;
  -    char buf[512], *cfname;
  +    char buf[MAX_STRING_LEN], *cfname;
       char *more_info;
       command_rec *cmd = NULL;
       handler_rec *hand = NULL;
  @@ -383,9 +381,7 @@
           if (!r->args) {
               rputs("<tt><a href=\"#server\">Server Settings</a>, ", r);
               for (modp = top_module; modp; modp = modp->next) {
  -                ap_snprintf(buf, sizeof(buf),
  -                          "<a href=\"#%s\">%s</a>", modp->name, modp->name);
  -                rputs(buf, r);
  +                rprintf(r, "<a href=\"#%s\">%s</a>", modp->name, modp->name);
                   if (modp->next) {
                       rputs(", ", r);
                   }
  @@ -394,102 +390,68 @@
   
           }
           if (!r->args || !strcasecmp(r->args, "server")) {
  -            ap_snprintf(buf, sizeof(buf),
  -                      "<a name=\"server\"><strong>Server Version:</strong> "
  +            rprintf(r, "<a name=\"server\"><strong>Server Version:</strong> "
                           "<font size=+1><tt>%s</tt></a></font><br>\n",
                           SERVER_VERSION);
  -            rputs(buf, r);
  -            ap_snprintf(buf, sizeof(buf),
  -                        "<strong>Server Built:</strong> "
  +            rprintf(r, "<strong>Server Built:</strong> "
                           "<font size=+1><tt>%s</tt></a></font><br>\n",
                           SERVER_BUILT);
  -            rputs(buf, r);
  -            ap_snprintf(buf, sizeof(buf),
  -                        "<strong>API Version:</strong> "
  +            rprintf(r, "<strong>API Version:</strong> "
                           "<tt>%d</tt><br>\n",
                           MODULE_MAGIC_NUMBER);
  -            rputs(buf, r);
  -            ap_snprintf(buf, sizeof(buf),
  -                        "<strong>Run Mode:</strong> <tt>%s</tt><br>\n",
  +            rprintf(r, "<strong>Run Mode:</strong> <tt>%s</tt><br>\n",
                           (standalone ? "standalone" : "inetd"));
  -            rputs(buf, r);
  -            ap_snprintf(buf, sizeof(buf),
  -                        "<strong>User/Group:</strong> "
  +            rprintf(r, "<strong>User/Group:</strong> "
                           "<tt>%s(%d)/%d</tt><br>\n",
                           user_name, (int) user_id, (int) group_id);
  -            rputs(buf, r);
  -            ap_snprintf(buf, sizeof(buf),
  -                        "<strong>Hostname/port:</strong> "
  +            rprintf(r, "<strong>Hostname/port:</strong> "
                           "<tt>%s:%u</tt><br>\n",
                           serv->server_hostname, serv->port);
  -            rputs(buf, r);
  -            ap_snprintf(buf, sizeof(buf),
  -                        "<strong>Daemons:</strong> "
  +            rprintf(r, "<strong>Daemons:</strong> "
                           "<tt>start: %d &nbsp;&nbsp; "
                           "min idle: %d &nbsp;&nbsp; "
                           "max idle: %d &nbsp;&nbsp; "
                           "max: %d</tt><br>\n",
                           daemons_to_start, daemons_min_free,
                           daemons_max_free, daemons_limit);
  -            rputs(buf, r);
  -            ap_snprintf(buf, sizeof(buf),
  -                        "<strong>Max Requests:</strong> "
  +            rprintf(r, "<strong>Max Requests:</strong> "
                           "<tt>per child: %d &nbsp;&nbsp; "
                           "keep alive: %s &nbsp;&nbsp; "
                           "max per connection: %d</tt><br>\n",
                           max_requests_per_child,
                           (serv->keep_alive ? "on" : "off"),
                           serv->keep_alive_max);
  -            rputs(buf, r);
  -            ap_snprintf(buf, sizeof(buf),
  -                        "<strong>Threads:</strong> "
  +            rprintf(r, "<strong>Threads:</strong> "
                           "<tt>per child: %d &nbsp;&nbsp; </tt><br>\n",
                           threads_per_child);
  -            rputs(buf, r);
  -            ap_snprintf(buf, sizeof(buf),
  -                        "<strong>Excess requests:</strong> "
  +            rprintf(r, "<strong>Excess requests:</strong> "
                           "<tt>per child: %d &nbsp;&nbsp; </tt><br>\n",
                           excess_requests_per_child);
  -            rputs(buf, r);
  -            ap_snprintf(buf, sizeof(buf),
  -                        "<strong>Timeouts:</strong> "
  +            rprintf(r, "<strong>Timeouts:</strong> "
                           "<tt>connection: %d &nbsp;&nbsp; "
                           "keep-alive: %d</tt><br>",
                           serv->timeout, serv->keep_alive_timeout);
  -            rputs(buf, r);
  -            ap_snprintf(buf, sizeof(buf),
  -                        "<strong>Server Root:</strong> "
  +            rprintf(r, "<strong>Server Root:</strong> "
                           "<tt>%s</tt><br>\n", server_root);
  -            rputs(buf, r);
  -            ap_snprintf(buf, sizeof(buf),
  -                        "<strong>Config File:</strong> "
  +            rprintf(r, "<strong>Config File:</strong> "
                           "<tt>%s</tt><br>\n", server_confname);
  -            rputs(buf, r);
  -            ap_snprintf(buf, sizeof(buf),
  -                        "<strong>PID File:</strong> "
  +            rprintf(r, "<strong>PID File:</strong> "
                           "<tt>%s</tt><br>\n", pid_fname);
  -            rputs(buf, r);
  -            ap_snprintf(buf, sizeof(buf),
  -                        "<strong>Scoreboard File:</strong> "
  +            rprintf(r, "<strong>Scoreboard File:</strong> "
                           "<tt>%s</tt><br>\n", scoreboard_fname);
  -            rputs(buf, r);
           }
           rputs("<hr><dl>", r);
           for (modp = top_module; modp; modp = modp->next) {
               if (!r->args || !strcasecmp(modp->name, r->args)) {
  -                ap_snprintf(buf, sizeof(buf),
  -                         "<dt><a name=\"%s\"><strong>Module Name:</strong> "
  +                rprintf(r, "<dt><a name=\"%s\"><strong>Module Name:</strong> "
                               "<font size=+1><tt>%s</tt></a></font>\n",
                               modp->name, modp->name);
  -                rputs(buf, r);
                   rputs("<dt><strong>Content handlers:</strong>", r);
                   hand = modp->handlers;
                   if (hand) {
                       while (hand) {
                           if (hand->content_type) {
  -                            ap_snprintf(buf, sizeof(buf),
  -                                      " <tt>%s</tt>\n", hand->content_type);
  -                            rputs(buf, r);
  +                            rprintf(r, " <tt>%s</tt>\n", hand->content_type);
                           }
                           else {
                               break;
  @@ -617,10 +579,9 @@
                   if (cmd) {
                       while (cmd) {
                           if (cmd->name) {
  -                            ap_snprintf(buf, sizeof(buf),
  -                                        "<dd><tt>%s - <i>",
  -                                        mod_info_html_cmd_string(cmd->name));
  -                            rputs(buf, r);
  +                            rprintf(r, "<dd><tt>%s - <i>",
  +				    mod_info_html_cmd_string(cmd->name,
  +					buf, sizeof(buf)));
                               if (cmd->errmsg) {
                                   rputs(cmd->errmsg, r);
                               }