Dear Apache-httpd developers: I am a Ph.D student in the Software Engineering Research Group in Case Western Reserve University, under the instruction of Prof. Andy Podgurski. In our recent research we analyzed some of your fixed bugs in your issue data base as well as some revisions which indicate fixing a bug, and try to find out whether there are similar bugs left in the code base which are left unfixed. We applied our approach in your newest release 2.2.13 and we have identified a few potential bugs as follows. It would be greatly appreciated if you can reply to this email after you have gone over the bugs and tell us whether you have confirmed any of them, since these information are really valuable for us for testing our current method.
1. Analyzed bug-fix: 39722 (https://issues.apache.org/bugzilla/show_bug.cgi?id=39722) This bug-fix checks return value of “ap_server_root_relative”; we have found two potential bugs where the return value of the function is not checked **************************original bug-fix********************************** Filename: server/core.c /* Make it absolute, relative to ServerRoot */ arg = ap_server_root_relative(cmd->pool, arg); + if (arg == NULL) { + return "DocumentRoot must be a directory"; + } **************************discovered possible new bug(s)*********************** (1) Filename: server/scoreboard.c 215 fname = ap_server_root_relative(pconf, ap_scoreboard_fname); 216 217 return create_namebased_scoreboard(global_pool, fname); (2) Filename: server/config.c 1654 && !(strcmp(fname, ap_server_root_relative(p, SERVER_CONFIG_FILE)))) { 1655 apr_finfo_t finfo; 2. Analyzed bug-fix: 39518 (://issues.apache.org/bugzilla/show_bug.cgi?id=39518) This bug-fix changes “apr_palloc” / “memcpy” construction into a single “apr_pmemdup”. We have discovered some “apr_palloc” / “memcpy” pair which should be changed into one “apr_pmemdup” function. **************************original bug-fix********************************** Filename: modules/filters/mod_include.c - char *to_release = apr_palloc(ctx->pool, release); + char *to_release = apr_pmemdup(ctx->pool, intern->start_seq, release); - memcpy(to_release, intern->start_seq, release); **************************discovered possible new bug(s)*********************** (1) Filename: server/request.c 761 buf = apr_palloc(r->pool, buflen); 762 memcpy(buf, r->filename, filename_len + 1); (2) Filename: server/util_filter.c 77 new = (filter_trie_child_ptr *)apr_palloc(p, parent->size * 78 sizeof(filter_trie_child_ptr)); 79 memcpy(new, parent->children, parent->nchildren * 80 sizeof(filter_trie_child_ptr)); (3) Filename: server/protocal.c 746 fold_buf = (char *)apr_palloc(r->pool, alloc_len); 747 memcpy(fold_buf, last_field, last_len); (4) Filename: server/protocol.c 302 new_buffer = apr_palloc(r->pool, new_size); 303 304 /* Copy what we already had. */ 305 memcpy(new_buffer, *s, bytes_handled); (5) Filename: server/protocal.c 417 new_buffer = apr_palloc(r->pool, new_size); 418 419 /* Copy what we already had. */ 420 memcpy(new_buffer, *s, bytes_handled); (6) Filename: modules/filters/mod_include.c 2426 kv_length = k_len + v_len + sizeof("=\n"); 2427 key_val = apr_palloc(ctx->pool, kv_length); 2428 next = key_val; 2429 2430 memcpy(next, key_text, k_len); (7) Filename: modules/generators/mod_autoindex.c 2188 fullpath = apr_palloc(r->pool, APR_PATH_MAX); 2189 dirpathlen = strlen(name); 2190 memcpy(fullpath, name, dirpathlen); (8) Filename: modules/generators/mod_autoindex.c 1545 name_scratch = apr_palloc(r->pool, name_width + 1); … 1722 else { 1723 nwidth = strlen(t2); 1724 if (nwidth > name_width) { 1725 memcpy(name_scratch, t2, name_width - 3); … 1726 nwidth = strlen(t2); 1727 if (nwidth > name_width) { 1728 memcpy(name_scratch, t2, name_width - 3); *DESCRIPTION*: line 1521 and 1698 could be combine into one pr_pmemdup; line 1521 and line 1783 could also be combined. (9) Filename: modules/http/mod_mime.c 185 exinfo = (extension_info*)apr_palloc(p, sizeof(*exinfo)); 186 apr_hash_set(mappings, suffix[i].name, 187 APR_HASH_KEY_STRING, exinfo); 188 memcpy(exinfo, copyinfo, sizeof(*exinfo)); 3. Analyzed bug-fix: revision 602467 The log of this revision is as follows: “ * core log.c: Work around possible solutions rejected by apr for the old implementation of apr_proc_create(), and explicitly pass the output and error channels to all log processes created. This goes all the way back to piped logs failing to run on win32. Not in or needed at trunk/, as apr 1.3.0 has the proper fix.” So we believe that in the 2.2.x branch, “apr_procattr_child_out_set” and “apr_procattr_child_err_set” should be invoked to explicitly pass the output and error channels to all log processes before “apr_proc_create” is invoked. We’ve found one bug where the two “apr_procattr_child_*_set” functions are missing. **************************original bug-fix********************************** Filename: server/log.c + apr_file_t *outfile, *errfile; + + if ((status = apr_file_open_stdout(&outfile, pl->p)) == APR_SUCCESS) + status = apr_procattr_child_out_set(procattr, outfile, NULL); + if ((status = apr_file_open_stderr(&errfile, pl->p)) == APR_SUCCESS) + status = apr_procattr_child_err_set(procattr, errfile, NULL); apr_tokenize_to_argv(pl->program, &args, pl->p); pname = apr_pstrdup(pl->p, args[0]); apr_tokenize_to_argv(pl->program, &args, pl->p); pname = apr_pstrdup(pl->p, args[0]); . procnew = apr_pcalloc(pl->p, sizeof(apr_proc_t)); status = apr_proc_create(procnew, pname, (const char * const *) args, NULL, procattr, pl->p); **************************discovered possible new bug(s)*********************** Filename: modules\generators\mod_cgi.c 421 if (((rc = apr_procattr_create(&procattr, p)) != APR_SUCCESS) || 422 ((rc = apr_procattr_io_set(procattr, 423 e_info->in_pipe, 424 e_info->out_pipe, 425 e_info->err_pipe)) != APR_SUCCESS) || 426 ((rc = apr_procattr_dir_set(procattr, 427 ap_make_dirstr_parent(r->pool, 428 r->filename))) != APR_SUCCESS) || 429 #ifdef RLIMIT_CPU 430 ((rc = apr_procattr_limit_set(procattr, APR_LIMIT_CPU, 431 conf->limit_cpu)) != APR_SUCCESS) || 432 #endif 433 #if defined(RLIMIT_DATA) || defined(RLIMIT_VMEM) || defined(RLIMIT_AS) 434 ((rc = apr_procattr_limit_set(procattr, APR_LIMIT_MEM, 435 conf->limit_mem)) != APR_SUCCESS) || 436 #endif 437 #ifdef RLIMIT_NPROC 438 ((rc = apr_procattr_limit_set(procattr, APR_LIMIT_NPROC, 439 conf->limit_nproc)) != APR_SUCCESS) || 440 #endif 441 ((rc = apr_procattr_cmdtype_set(procattr, 442 e_info->cmd_type)) != APR_SUCCESS) || 443 444 ((rc = apr_procattr_detach_set(procattr, 445 e_info->detached)) != APR_SUCCESS) || 446 ((rc = apr_procattr_addrspace_set(procattr, 447 e_info->addrspace)) != APR_SUCCESS) || 448 ((rc = apr_procattr_child_errfn_set(procattr, cgi_child_errfn)) != APR_SUCCESS)) { 449 /* Something bad happened, tell the world. */ 450 ap_log_rerror(APLOG_MARK, APLOG_ERR, rc, r, 451 "couldn't set child process attributes: %s", r->filename); 452 } 453 else { 454 procnew = apr_pcalloc(p, sizeof(*procnew)); 455 rc = ap_os_create_privileged_process(r, procnew, command, argv, env, 456 procattr, p); Filename: os/unix/unixd.c Function: ap_os_create_privileged_process 409 if (ugid == NULL) { 410 return apr_proc_create(newproc, progname, args, env, attr, p); 411 } 412 413 return ap_unix_create_privileged_process(newproc, progname, args, env, 414 attr, ugid, p); *DESCRIPTION*: The parameter “procattr” is created in first code segment by line 421 and passed to the function “ap_os_create_privileged_process” at line 455; and in the function“ap_os_create_privileged_process”, “apr_proc_create” is invoked at line 410; “ap_unix_create_privileged_process” is invoked at line 413, in which “apr_proc_create” is also invoked. So we think that “apr_procattr_child_out_set” and “apr_procattr_child_err_set” is missing in the file modules\generators\mod_cgi.c before “ap_os_create_privileged_process” is called in line 455. Another evidence of this bug is that in another file “modules\generators\mod_cgid.c”, “apr_procattr_child_out_set” and “apr_procattr_child_err_set” did invoked before “ap_os_create_privileged_process”, as follows: 748 if (((rc = apr_procattr_create(&procattr, ptrans)) != APR_SUCCESS) || ……. 758 ((rc = apr_procattr_child_err_set(procattr, r->server->error_log, NULL)) != APR_SUCCESS) || 759 ((rc = apr_procattr_child_in_set(procattr, inout, NULL)) != APR_SUCCESS))) || …… 790 rc = ap_os_create_privileged_process(r, procnew, argv0, argv, Thank you very much in advance! Boya Sun Computer Science Division Electrical Engineering & Computer Science Department 513 Olin Building Case Western Reserve University 10900 Euclid Avenue Clevelnd, OH 44106 [email protected] 2009-08-19
