Created attachment 27993 [details] Patch (check NULLness of name before use) Last Friday, a SAMSUNG-GT-S5230 stumbled across one of our servers and sent the following Cookie Header (truncated): Cookie: =; facdd=1; facdp=240#947; facdo=portrait Since we do log one cookies on this specific server via a LogFormat with NAME=\"%{COOKIENAME}C\", this caused a segfault in apr_collapse_spaces, called from log_cookie. In log_cookie(), name is set to NULL for the nameless and valueless cookie, which causes just about everything in the following block to segfault. (Line numbers don't align with stock httpd 2.2.21 due an unrelated custom patch in log_env_var()). #0 apr_collapse_spaces (dest=0x0, src=0x0) at strings/apr_cpystrn.c:220 220 while (*src) { (gdb) bt #0 apr_collapse_spaces (dest=0x0, src=0x0) at strings/apr_cpystrn.c:220 #1 0x000000000046b4a4 in log_cookie (r=0xcac1790, a=0xaf048d0 "COOKIENAME") at mod_log_config.c:561 #2 0x000000000046ab94 in process_item (r=<value optimized out>, cls=0x9ffdde8, default_format=<value optimized out>) at mod_log_config.c:976 #3 config_log_transaction (r=<value optimized out>, cls=0x9ffdde8, default_format=<value optimized out>) at mod_log_config.c:1039 #4 0x000000000046acb3 in multi_log_transaction (r=0x0) at mod_log_config.c:1070 #5 0x00000000004363ea in ap_run_log_transaction (r=0xcac1790) at protocol.c:1696 #6 0x00000000004a9e5c in ap_process_request (r=0xcac1790) at http_request.c:308 #7 0x00000000004a69e0 in ap_process_http_connection (c=0xb45c1b0) at http_core.c:190 #8 0x000000000044e21a in ap_run_process_connection (c=0xb45c1b0) at connection.c:43 #9 0x00000000004e1330 in child_main (child_num_arg=<value optimized out>) at prefork.c:667 #10 0x00000000004e1741 in make_child (s=0x86e4710, slot=38) at prefork.c:771 #11 0x00000000004e1fb8 in ap_mpm_run (_pconf=<value optimized out>, plog=<value optimized out>, s=<value optimized out>) at prefork.c:906 #12 0x000000000042fae6 in main (argc=3, argv=0x7fff3072ecc8) at main.c:739 (gdb) up #1 0x000000000046b4a4 in log_cookie (r=0xcac1790, a=0xaf048d0 "COOKIENAME") at mod_log_config.c:561 561 apr_collapse_spaces(name, name);
Doesn't that mean that if the name is now empty you don't escape the value either? - Or am I misreading this?
(In reply to comment #1) > Doesn't that mean that if the name is now empty you don't escape the value > either? - Or am I misreading this? I assume you are misreading the patch. If the name is empty, nothing will be returned for this cookie, the return ap_escape_logitem() is never called. Instead, the while loop simply proceeds with the next cookie. If no other cookie matches, NULL is returned at the exit of the function. Cookies with empty names are illegal according to rfc2109, so there's no need to consider the case that someone might want to log such things with %{}C.
trunk: r1225380 2.4: r1225385
backported to 2.2: r1227292 And Assigned as CVE-2012-0021
2.2.22 is released