httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From 孙搏雅 <sunb...@gmail.com>
Subject bugs discovered by a research group at CWRU
Date Wed, 19 Aug 2009 17:04:53 GMT
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
boya.sun@case.edu
2009-08-19

Mime
View raw message