I have been able to get the following error messages in httpd-error.log by modifying suexec.c just to see what would happen if execv() returned. [error] Premature end of script headers: hello.pl [error] failed to open log file [error] fopen: Permission denied There is a comment near the end of suexec.c's main() function which says that the log file will be closed so that CGI can't tamper with it. It also states that "If the exec fails, it will be reopened automatically when log_err is called." So this is what I wanted to test. I did the test by simply preceeding execv() with the "kaboom" log_err() statement that *would* be called if execv() returned. Here's a diff of what I mean: --- suexec.c.orig Fri Jul 12 10:42:48 2002 +++ suexec.c Fri Jul 12 11:13:30 2002 @@ -620,6 +620,7 @@ ap_execve(cmd, &argv[3], environ); } #else /*NEED_HASHBANG_EMUL*/ + log_err("(%d)%s: exec failed (%s)\n", errno, strerror(errno), cmd); execv(cmd, &argv[3]); #endif /*NEED_HASHBANG_EMUL*/ I may be totally wrong by performing the test like this but as far as I see, if execv() ever returns, the "kaboom" line will not execute as expected because fopen will bomb. Please let me know if I am wrong about any of this.
I think the problem is that suexec can't reopen the logs because it already has dropped privileges. Perhaps setting the fd close-on-exec would be better than explicitly closing it, but I don't know if that is portable.
FWIW, this bug has been reported against Debian's apache package too and it includes a patch: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=153528 Thoughts?
the patch in the Debian BTS looks good.
(In reply to comment #3) > the patch in the Debian BTS looks good. But be sure to check the result of the call to fcntl(). If we did not succeed in setting the CLOEXEC flag, bad things may happen. Roland
(In reply to comment #4) > But be sure to check the result of the call to fcntl(). If we did not succeed in > setting the CLOEXEC flag, bad things may happen. ACK. How about this one? @@ -638,19 +642,15 @@ umask(SUEXEC_UMASK); #endif /* SUEXEC_UMASK */ - /* - * Be sure to close the log file so the CGI can't - * mess with it. If the exec fails, it will be reopened - * automatically when log_err is called. Note that the log - * might not actually be open if LOG_EXEC isn't defined. - * However, the "log" cell isn't ifdef'd so let's be defensive - * and assume someone might have done something with it - * outside an ifdef'd LOG_EXEC block. - */ if (log != NULL) { - fclose(log); - log = NULL; + fflush(log); + setbuf(log,NULL); + if(fcntl(fileno(log),F_SETFD,FD_CLOEXEC)==-1) { + log_err("error: can't set close-on-exec flag"); + exit(122); + } } + /* * Execute the command, replacing our image with its own.
Created attachment 20270 [details] A patch against suexec.c of Apache 2.2.4 Just a patch against the newest Apache. (In preparation for a posting on the httpd-dev mailinglist)
commited to trunk as r821321