Closed Bug 140784 Opened 22 years ago Closed 22 years ago
edit*.cgi need a use lib "." on Win32/IIS w/ taint
At least on W2k ActiveState default installation the edit*.cgi don't work, because they cannot find the required 'cgi.pl'. I'll attach a patch which adds 'use lib "."' to all the edit*.cgi files. This is the solution used in the other cgis as well.
Why is this needed? Those templates don't run in taint mode, so '.' should be part of the default lib path.
Oh yeah, I must've been really tired when writing that bug report. It's a much clearer one now that I've slept overnight. ;-) The bug is caused by a particular Microsoft IIS web server configuration and ActiveState Perl interpreter. In order to run CGIs in taint mode through IIS, you have to hardcode -T switch into the association command line configured into the IIS properties. This is needed for the user-wise cgi:s, but since the edit*.cgis reside in the same directory, they get the -T tagged on them too, and thus are run in taint mode. Shebang lines won't make much difference there. :-( So, while technically this isn't a Bugzilla bug, my fix proposal seems harmless security-wise and makes the Win32-IIS-combination much easier to use. Resummarized for the specifics noted.
Summary: edit*.cgi need a use lib "." → edit*.cgi need a use lib "." on Win32/IIS w/ taint
Ah. mod_perl has the same restriction. Does the edit* stuff run in taint mode, though? Hopefully there are no security faults, but there may be stuff which we check, but don't explicitly detaint - that was the problem in the majority of the user-facing scripts. The patch won't hurt, but when I did the taint stuff we decided that 2.16 would only do the user-facing pages, and that it would be done as part of rewriting the edit* stuff for 2.18. If they work, then we should add the -T option so that everyone gets the benefit, although thats risky at this stage of the release process.
Target Milestone: --- → Bugzilla 2.16
I had no problems with the edit* code even after applying the patch I attached and setting IIS to use a fixed -T parameter, but I did not check every possible alternative way of breaking them. Your proposal of adding the -T to shebang lines sounds sensible, if the edit*cgis really work 100%. But regardless of that, I think the use lib fix should be checked in separately. It has no harmful effect but benefits IIS and mod_perl users, and making admin cgis taint-checked is really a separate issue (is there already a bug on this? couldn't find one). Besides, 2.16 doesn't really need the potential unstability caused by taint issues. Nevertheless, unless you're going to do something about it, I can check out whether there are any hiding taint problems in edit* files. Hopefully later today.
I would be suprised if all of edit*.cgi work with taint as is. I suppose use lib . could be done for 2.16 but I don't think we should be adding taint to the shebang line without a fair bit of testing.
Comment on attachment 81419 [details] [diff] [review] Patch proposal v1 r=bbaetz. This patch can't hurt. Theres very little validation to be done, though, so if everywhere does use SendSQL (and for stuff like usernames, which can have spaces, we sould have noticed if this wasn't done), I wouldn't be surprised if it mostly worked. Maybe we'll just have to document that due to limitations in the way IIS and Perl interacts, taint mode can't be enabled on windows?
A more appropriate documentation note might be "Use taint mode under Windows and IIS at your own risk; some of the administrative pages may function incorrectly." The problem with leaving taint mode off is that you will have to edit all the user-wise cgis and remove the -Ts from their shebang lines. So it's source editing even that way. At least my current experience says that using taint is easier than not, especially after this patch is checked in. And additionally there's of course the safety benefit.
Perl on Win32 *does* read the shebang line for interpreter flags *at runtime*. -T can't be turned on that way due to the way it works though. That one has to be done on the command-line or it won't work at all. (Ever seen the "Too late for -T" error when running "perl -cw" instead of "perl -cWT" on a script with a -T on the shebang line?) But it wouldn't surprise me if a -T on the command-line was disabled by the lack of one on the shebang line on Win32. I will investigate later today when I have access to a Win32 machine to try it out on.
>But it wouldn't surprise me if a -T on the command-line was disabled by >the lack of one on the shebang line on Win32. Now that would surprise _me_ ;-) I filed this bug because my commandline -T also affected the edit*.cgi:s which certainly do not have -T's on their shebang lines. But I'll be happy to be corrected if I'm wrong; hopefully someone can then point out (and document) what one has to do to make both tainted and untainted files work with the same command-line parameters. Might there be difference between Perl implementations? My results were based on a couple-of-months-old ActiveState Perl binary.
Comment on attachment 81419 [details] [diff] [review] Patch proposal v1 r=gerv. Gerv
Fixed. Thanks again :-) Checking in editattachstatuses.cgi; /cvsroot/mozilla/webtools/bugzilla/editattachstatuses.cgi,v <-- editattachstatuses.cgi new revision: 1.7; previous revision: 1.6 done Checking in editcomponents.cgi; /cvsroot/mozilla/webtools/bugzilla/editcomponents.cgi,v <-- editcomponents.cginew revision: 1.22; previous revision: 1.21 done Checking in editgroups.cgi; /cvsroot/mozilla/webtools/bugzilla/editgroups.cgi,v <-- editgroups.cgi new revision: 1.16; previous revision: 1.15 done Checking in editkeywords.cgi; /cvsroot/mozilla/webtools/bugzilla/editkeywords.cgi,v <-- editkeywords.cgi new revision: 1.8; previous revision: 1.7 done Checking in editmilestones.cgi; /cvsroot/mozilla/webtools/bugzilla/editmilestones.cgi,v <-- editmilestones.cginew revision: 1.8; previous revision: 1.7 done Checking in editparams.cgi; /cvsroot/mozilla/webtools/bugzilla/editparams.cgi,v <-- editparams.cgi new revision: 1.14; previous revision: 1.13 done Checking in editproducts.cgi; /cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v <-- editproducts.cgi new revision: 1.24; previous revision: 1.23 done Checking in editusers.cgi; /cvsroot/mozilla/webtools/bugzilla/editusers.cgi,v <-- editusers.cgi new revision: 1.34; previous revision: 1.33 done Checking in editversions.cgi; /cvsroot/mozilla/webtools/bugzilla/editversions.cgi,v <-- editversions.cgi new revision: 1.11; previous revision: 1.10 done Gerv
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
I'll still add that I quickly went through all the edit files and found no taint problems. The rest of -T stuff _should_ thus be trivial... I couldn't find an existing bug on tainting of admin cgis, so I filed a new one, bug 141006.
and just as a followup, no, a missing -T on the shebang line does not disable taint on Win32.
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.