Closed Bug 140784 Opened 21 years ago Closed 21 years ago

edit*.cgi need a use lib "." on Win32/IIS w/ taint

Categories

(Bugzilla :: Administration, task)

2.15
x86
Windows 2000
task
Not set
major

Tracking

()

RESOLVED FIXED
Bugzilla 2.16

People

(Reporter: jouni, Assigned: justdave)

Details

Attachments

(1 file)

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.
Keywords: patch, review
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?
Attachment #81419 - Flags: review+
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
Attachment #81419 - Flags: review+
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: 21 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.