Created attachment 33501 [details] minimal httpd config to reproduce the issue While playing around with httpd-2.4.18, I sent an (invalid) HTTP CONNECT request with a URL instead of a host:port to the httpd ("CONNECT /index.html HTTP/1.1") what resulted in immediate drop of the connection. After investigation I found that the httpd process actually crashed (segfault). The core reveals that the r->protocol pointer is 0x0, which is used without check in an strcmp() call in ap_add_cgi_vars(). Program terminated with signal SIGSEGV, Segmentation fault. #0 0x000055fdfff85243 in ap_add_cgi_vars (r=0x7f2f98003eb0) at util_script.c:382 382 if (!strcmp(r->protocol, "INCLUDED")) { [Current thread is 1 (Thread 0x7f2fa537d700 (LWP 31095))] (gdb) bt #0 0x000055fdfff85243 in ap_add_cgi_vars (r=0x7f2f98003eb0) at util_script.c:382 #1 0x00007f2fa5b84314 in includes_filter (f=<optimized out>, b=<optimized out>) at mod_include.c:3887 #2 0x000055fdfff6b762 in default_handler (r=0x7f2f98003eb0) at core.c:4517 #3 0x000055fdfff7d1b0 in ap_run_handler (r=r@entry=0x7f2f98003eb0) at config.c:169 #4 0x000055fdfff7d6f9 in ap_invoke_handler (r=r@entry=0x7f2f98003eb0) at config.c:433 #5 0x000055fdfff94a4c in ap_internal_redirect (new_uri=<optimized out>, r=<optimized out>) at http_request.c:730 #6 0x000055fdfff67fbb in ap_read_request (conn=conn@entry=0x7f2fa000bfb8) at protocol.c:985 #7 0x000055fdfff91a09 in ap_process_http_async_connection (c=0x7f2fa000bfb8) at http_core.c:146 #8 ap_process_http_connection (c=0x7f2fa000bfb8) at http_core.c:248 #9 0x000055fdfff86ff0 in ap_run_process_connection (c=c@entry=0x7f2fa000bfb8) at connection.c:41 #10 0x00007f2fa5d9357d in process_socket (my_thread_num=<optimized out>, my_child_num=0, cs=0x7f2fa000bf28, sock=<optimized out>, p=<optimized out>, thd=<optimized out>) at event.c:1101 #11 worker_thread (thd=<optimized out>, dummy=<optimized out>) at event.c:1960 #12 0x00007f2fa653f314 in start_thread () from /lib/libpthread.so.0 #13 0x00007f2fa608283d in clone () from /lib/libc.so.6 I tried to create a simple httpd.conf to reproduce the problem, and found that mod_include has to be active and an "ErrorDocument 400" directive pointing to an actual document has to exist. since the actual NULL pointer dereference happens in the function ap_add_cgi_vars() which is called from some other points in the code, it is quite possible that other ways to trigger this issue exists. The issue still exists in the 2.4 HEAD but was not reproducible with 2.4.17. to reproduce: - start apache with given httpd.conf. 400.html has to exist - send "CONNECT /index.html HTTP/1.1" -> server crashes
I suspect that the bug is this "early return" in protocol.c / read_request_line(): if (r->status != HTTP_OK) { return 0; } r->proto_num and r->protocol are not initialized in this case. This change has been introduced in r1710095.
(In reply to Michael Kaufmann from comment #1) > I suspect that the bug is this "early return" in protocol.c / > read_request_line(): > > if (r->status != HTTP_OK) { > return 0; > } > > r->proto_num and r->protocol are not initialized in this case. > > This change has been introduced in r1710095. Good analysis. Can you please try if the following patch fixes your issue: Index: server/protocol.c =================================================================== --- server/protocol.c (revision 1727166) +++ server/protocol.c (working copy) @@ -638,6 +638,8 @@ ap_parse_uri(r, uri); if (r->status != HTTP_OK) { + r->proto_num = HTTP_VERSION(1,0); + r->protocol = apr_pstrdup(r->pool, "HTTP/1.0"); return 0; }
(In reply to Ruediger Pluem from comment #2) Yes, this patch works for me.
I like the patch, it solves the bug where it was introduced. I'd additionally suggest to fix the ap_add_cgi_vars() function, to make it more robust. See patch bellow: --- a/server/util_script.c +++ b/server/util_script.c @@ -379,7 +379,7 @@ AP_DECLARE(void) ap_add_cgi_vars(request_rec *r) * come with the script URI in the include command. Ugh. */ - if (!strcmp(r->protocol, "INCLUDED")) { + if ( r->protocol && !strcmp(r->protocol, "INCLUDED")) { apr_table_setn(e, "SCRIPT_NAME", r->uri); if (r->path_info && *r->path_info) { apr_table_setn(e, "PATH_INFO", r->path_info);
Committed to trunk as r1727544
(In reply to Ruediger Pluem from comment #5) > Committed to trunk as r1727544 Thank you! Can you please propose this bugfix for the 2.4.x branch?
(In reply to Michael Kaufmann from comment #6) > (In reply to Ruediger Pluem from comment #5) > > Committed to trunk as r1727544 > > Thank you! Can you please propose this bugfix for the 2.4.x branch? Proposed for backport as r1728354
Thank you Ruediger! This bug is fixed in Apache httpd 2.4.20 (backported in r1729875).