Project

General

Profile

Actions
Copy link

Bug #2302

closed

sloppy error handling in mod_cgi to affect binaries

Added by kahic over 13 years ago. Updated almost 9 years ago.

Status:
Fixed
Priority:
Normal
Category:
mod_cgi
Target version:
1.4.40
ASK QUESTIONS IN Forums:

Description

For reasons unknown to me it appears that when preparing to call a cgi binary (for example a C or C++ program compiled with gcc) from mod_cgi directly (i.e. without an intermediate handler), there is no server socket object available in cgi_create_env.
This condition (a NULL pointer) is not in any way handled gracefully in the module, and as a consequence the child process crashes without any error report.
Furthermore, it does not call the binary, so the user is left in a very difficult position to debug the problem.
After I hurdled my way around this, I noticed another issue:
When failing to execve, the default action is to segfault, which leaves ugly traces in the syslog, but does not help the user either.
I attached a patch (against 1.4.26, the version shipped with the current Fedora release) to remedy the situation and ease the pain somewhat.
Do with it whatever you like.
At least my cgi programs appear to work now the way I would expect them to.

Files

mod_cgi_c.patch (2.18 KB) mod_cgi_c.patch kahic, 2011-03-12 13:57
Actions
Copy link
 #1

Updated by gstrauss almost 9 years ago

con->srv_socket is set soon after accept() and so it should never be NULL in mod_cgi. Is this reproducible in 1.4.x latest? Probably not.

I agree that segfaulting after exec failure is not very user friendly.
Submitted pull request: https://github.com/lighttpd/lighttpd1.4/pull/21

Actions
Copy link
 #2

Updated by stbuehler almost 9 years ago

  • Target version set to 1.4.40

Ignoring NULL-pointers is not the solution; if there is a good reason in a single case to expect a NULL-pointer, then there needs to be a proper handling of it, and proper handling might be to skip setting the cgi environment variable - but not like this added everywhere without justification.

I'll go with the patch from gstrauss. I don't see a big problem with a SEGFAULT here (because there really is something broken, and it increases the chances the problem shows up in some log), but logging the actual error is better of course.

Actions
Copy link
 #3

Updated by stbuehler almost 9 years ago

  • Status changed from New to Fixed
  • % Done changed from 50 to 100

Applied in changeset r3079.

Actions
Copy link

Also available in: Atom