1232297 - Bugzilla should require taint mode at runtime

Status: RESOLVED WONTFIX

(Bugzilla :: Bugzilla-General, defect, P1)

Description

[Dylan Hardison [:dylan] (he/him)]

We need to require taint mode (at runtime) until such time as we don't generate SQL by hand. This is as simple as checking ${^TAINT}.
I'm setting this as a blocker for PSGI support because PSGI makes it very easy to run bugzilla without taint mode.

Comment 1

[Dylan Hardison [:dylan] (he/him)]

Attachment: 1232297_1.patch

Comment 2

[Frédéric Buclin]

I don't think we need to *require* taint mode. If one wants to run his Bugzilla installation with taint mode disabled, that's his own responsibility. Remember you wanted to do it for BMO.

Comment 3

[Dylan Hardison [:dylan] (he/him)]

What about if we require this unless a param is specifically set in data/params? I would like it to be impossible to accidentally turn off taint mode. Currently it is difficult because of the apache config and/or the shebang lines, but this goes away under PSGI.

Comment 4

[Frédéric Buclin]

(In reply to Dylan William Hardison [:dylan] from comment #3)
> What about if we require this unless a param is specifically set in
> data/params?

A parameter is overkill, especially because you would have to restart your web server anyway to enable/disable the taint mode. An environment variable would be preferable IMO, e.g. BZ_NO_TAINT_OK = 1. When set to a true value, no check against ${^TAINT} would be done. If false or missing, then the taint mode would be required.

Comment 5

[Dylan Hardison [:dylan] (he/him)]

Attachment: 1232297_2.patch

Comment 6

[Dylan Hardison [:dylan] (he/him)]

Attachment: 1232297_3.patch

Comment 7

[Frédéric Buclin]

Comment on attachment 8702433 [details] [diff] [review]
1232297_3.patch

>+++ b/Bugzilla.pm

>+    ThrowCodeError('taint_mode_required') if i_am_cgi() && !${^TAINT} && !$ENV{BZ_DISABLE_TAINT_MODE};

When using plackup, this code is called by app.psgi itself, not by CGI scripts, and so i_am_cgi() is always false.
Also, this line is too long, see bug 988736.


>+++ b/template/en/default/global/code-error.html.tmpl

>+  [% ELSIF error == "taint_mode_required" %]
>+    [% terms.Bugzilla %] requires taint mode to be enabled. If you really want to run [% terms.Bugzilla %]
>+    without taint mode, please set the <code>$BZ_DISABLE_TAINT_MODE</code> environmental variable to "1".

s/environmental/environment/ per http://perldoc.perl.org/Env.html. Also, remove $ in front of the variable name; it's confusing IMO.
These lines are too long. They should not exceed 80 characters, see above.

Comment 8

[Dylan Hardison [:dylan] (he/him)]

Fixing this would preclude running bugzilla under plack, as plack has zero support for running under taint mode.