Re: svn commit: r1534015 - /httpd/httpd/trunk/server/main.c

2013-10-23 Thread Yann Ylavic
On Wed, Oct 23, 2013 at 1:27 AM, William A. Rowe Jr. wmr...@gmail.comwrote:


 On Oct 22, 2013 5:14 PM, Yann Ylavic ylavic@gmail.com wrote:
 
 
  Shouldn't this be safe from terminal controls, eg :
  const char *name = process-short_name;
  if (!name ||
  !*name ||
  ap_has_cntrl(name)) {
  name = httpd;
  }
  ?

 No.  You are thinking of untrusted user input.  The Admin started this
 process under the given name.  Describe how this can be devolved to a
 vulnerability?

No particular vulnerability (in sane circumstances, ie. the Admin is not
given an evil name), and not an httpd vulnerability anyway, but the usual
ap_log_error(...STARTUP...) does escape control chars, which make this code
the only place where some given data is put direclty to the terminal. I
can probably live with it...

Regards.


Re: svn commit: r1534015 - /httpd/httpd/trunk/server/main.c

2013-10-22 Thread Yann Ylavic
On Mon, Oct 21, 2013 at 2:30 AM, n...@apache.org wrote:

 Author: niq
 Date: Mon Oct 21 00:30:26 2013
 New Revision: 1534015

 URL: http://svn.apache.org/r1534015
 Log:
 Fix r55670.  Not a great idea to dereference process after pool destroy!

 Modified:
 httpd/httpd/trunk/server/main.c

 Modified: httpd/httpd/trunk/server/main.c
 URL:
 http://svn.apache.org/viewvc/httpd/httpd/trunk/server/main.c?rev=1534015r1=1534014r2=1534015view=diff

 ==
 --- httpd/httpd/trunk/server/main.c (original)
 +++ httpd/httpd/trunk/server/main.c Mon Oct 21 00:30:26 2013
 @@ -264,12 +264,17 @@ static void destroy_and_exit_process(pro
   * by us before they can do so. In this case maybe valueable log
 messages
   * might get lost.
   */
 +
 +/* If we are to print an error, we need the name before we destroy
 pool.
 + * short_name is a pointer into argv, so remains valid.
 + */
 +const char *name = process-short_name ? process-short_name :
 httpd;
 +


Shouldn't this be safe from terminal controls, eg :
const char *name = process-short_name;
if (!name ||
!*name || 
ap_has_cntrl(name)) {
name = httpd;
}
?


  apr_sleep(TASK_SWITCH_SLEEP);
  ap_main_state = AP_SQ_MS_EXITING;
  apr_pool_destroy(process-pool); /* and destroy all descendent pools
 */
  apr_terminate();
  if ((process_exit_value != 0)  isatty(fileno(stderr))) {
 -const char *name = process-short_name ? process-short_name :
 httpd;
  fprintf(stderr, %s: abnormal exit %d\n, name,
 process_exit_value);
  }
  exit(process_exit_value)


Regards.


Re: svn commit: r1534015 - /httpd/httpd/trunk/server/main.c

2013-10-22 Thread William A. Rowe Jr.
On Oct 22, 2013 5:14 PM, Yann Ylavic ylavic@gmail.com wrote:


 Shouldn't this be safe from terminal controls, eg :
 const char *name = process-short_name;
 if (!name ||
 !*name ||
 ap_has_cntrl(name)) {
 name = httpd;
 }
 ?

No.  You are thinking of untrusted user input.  The Admin started this
process under the given name.  Describe how this can be devolved to a
vulnerability?