Project

General

Profile

Actions

Bug #2781

closed

Tainted environment variable $SHELL when proc_open called

Added by viennadd about 7 years ago. Updated about 7 years ago.

Status:
Invalid
Priority:
Normal
Category:
core
Target version:
-
ASK QUESTIONS IN Forums:

Description

In function proc_open https://github.com/lighttpd/lighttpd1.4/blob/master/src/proc_open.c#L241

if (NULL == (shell = getenv(SHELLENV))) {                  // if environment variable $SHELL changed somehow, it might cause an unexpected problem.
shell = "/bin/sh";
}
...........
execl(shell, shell, "-c", command, (char *)NULL); // execute whatever contained in $SHELL
# problem demo
➜  src git:(master) ✗ export SHELL=/bin/cat
➜  src git:(master) ✗ ./lighttpd -f lighttpd.conf
/bin/cat: invalid option -- 'c'
Actions #1

Updated by stbuehler about 7 years ago

  • Status changed from New to Invalid
  • Target version deleted (1.4.x)

I see no problem relying on the start environment; if you as admin think cat is the proper SHELL for you, then that is your problem.

Actions #2

Updated by gstrauss about 7 years ago

environment variable taint checking is an excellent idea for a setuid program.

However, lighttpd is not such a program. In fact, lighttpd checks issetugid() and, if true, exits with the following message, and I quote:
"Are you nuts ? Don't apply a SUID bit to this binary"

Needless to say, this bug report is invalid.

BTW, proc_open is used only during processing of config files, to handle inclue_shell, and nowhere else in lighttpd.
On the other hand, mod_cgi safely gives CGI scripts a clean environment, not passing anything from the lighttpd server startup environment.

What static analysis tool were you using that reported this use of tainted environment variable? Perhaps you can share the report so that we can review it, instead of responding to one issue report at a time and wading through lots of false positives (please don't do the latter).

Actions #3

Updated by viennadd about 7 years ago

gstrauss wrote:

environment variable taint checking is an excellent idea for a setuid program.

However, lighttpd is not such a program. In fact, lighttpd checks issetugid() and, if true, exits with the following message, and I quote:
"Are you nuts ? Don't apply a SUID bit to this binary"

Needless to say, this bug report is invalid.

BTW, proc_open is used only during processing of config files, to handle inclue_shell, and nowhere else in lighttpd.
On the other hand, mod_cgi safely gives CGI scripts a clean environment, not passing anything from the lighttpd server startup environment.

What static analysis tool were you using that reported this use of tainted environment variable? Perhaps you can share the report so that we can review it, instead of responding to one issue report at a time and wading through lots of false positives (please don't do the latter).

Really thanks for your advices,
Yes, proc_open called only to handle include_shell, I was trying to confirm with you that $SHELL is reliable (and it is). I should describe more in the first place to save time.

My job is picking suspicious case from all reports, perhaps I can do better before I start an issue.

Thanks @gstrauss, @stbuehler

Actions #4

Updated by gstrauss about 7 years ago

I was trying to confirm with you

DO NOT file bug reports to ask questions. Use the Forums: https://redmine.lighttpd.net/projects/lighttpd/boards

Please do not file any additional bug reports unless you have confirmed that it is a bug.
Instead, questions can be asked in the forums.

Actions #5

Updated by viennadd about 7 years ago

gstrauss wrote:

I was trying to confirm with you

DO NOT file bug reports to ask questions. Use the Forums: https://redmine.lighttpd.net/projects/lighttpd/boards

Please do not file any additional bug reports unless you have confirmed that it is a bug.
Instead, questions can be asked in the forums.

Noticed, sorry for bring inconvenience

Actions

Also available in: Atom