Problem statement

Recap from last time - BatchUploadPlugin generates the following error

"Insecure dependency in chmod while running with -T switch at /usr/lib/perl5/site_perl/5.8.8/Archive/Zip/Member.pm line 399. at /usr/lib/perl5/site_perl/5.8.8/Archive/Zip/Member.pm line 399"

Background and related info

I know that the '-T' switch turns on 'taint' checking (for more info on options that can be passed to the perl executable, see Perldoc:perlrun). Started out by searching at TWiki for the combination of BatchUploadPlugin and taint. Even when looking in all public webs, still only got two hits, which dropped me squarely back to BatchUploadPlugin is supposed to be the answer to this problem. Huh.

Read through Perldoc:perlsec again. Must say that it isn't all that helpful in trying to diagnose where the system is concerned about a potential security hole.

Info extracted from that page relevant to our situation really seems to come down to:

1. a variable could contain tainted data, and it's use could trigger an 'insecure dependency' error;
2. something we are calling could be doing something it shouldn't do, generating an "insecure $ENV{PATH}" message.
3. a tainted string has been added to @INC, resulting in the following problem: "Insecure dependency in require while running with -T switch")

Inquiry

First question is which of the possibilities do I start exploring? Unfortunately, the perlsec topic really is awfully dense - I can't tell if I'm supposed to be taking the examples literally or not. Well no, I'm being too generous. The frustration is that I clearly have to be generalizing at least one of the examples, and there's no guidance given as to which one.

Perldoc:perlsec contains extensive discussion concerning uids and gids (user and group ids), however, I consider problems related to ownership of the files and authorization into TWiki to be less probable as uploading a single file does not trigger the error. That makes it more likely that it has to do with the involvement of Archive::Zip, of chmod, or of path stuff.

1. start with simple path issues, as checking that is also fast (as was uploading a single file...)
   1. check the configuration
      • $ENV{PATH} is set (I checked the path using the TWiki configure interface)
      • the directories (/bin:/usr/bin) are absolute
      • neither directory is world-writable
   2. hmmmm, looks ok, but I noticed that some optional modules listed in the expert settings are not installed.
   3. might as well install the optional modules since I'm here.
      • pretty quickly discover a problem with installing Unicode::MapUTF8, actually, the prereq, Unicode::Map8.
      • Looked up the module in question on http://search.cpan.org/, CPAN:Unicode::Map8. Viewed the bug reports and realized I needed to install this particular fix from a fedora source rather than through cpanp as I usually do. (At this time, this twiki install is still on perl 5.8.8, as I don't want to undertake a major upgrade during the quarter. We're a research lab at a master's comp with minimal funding. The cost of being community-oriented, although that does seem to be changing. We do actually have some funding these days...) Install fix via yum, go back to cpanp, all else installs just fine.
   4. restart server, try again, no change. not surprised.
2. ok, move on to more likely stuff. I guess the big question is does Archive::Zip work and play well with taint checking, independent of BatchUploadPlugin? I suspect that the answer is yes, and the problem is in BatchUploadPlugin itself, as Archive::Zip is pretty solid.
   1. took a look at attach and upload in bin. They only differ in that one calls upload and the other calls attach, both are methods in TWiki::UI::Upload
   2. not sure how upload in bin relates to handlers registered in BatchUploadPlugin. So off to look at the templates. hmmmmm, grep batch *attach* in the templates directory gets me attach.nat.tmpl.
      • ahah! lookie there, one mystery solved. The text for BatchUploadPlugin, "Extracts individual files in a zip archive", are showing in grey because they are actually specified as being in twikiGrayText. Geesh. You know what? I don't even want to know. I just customize that away in attach.ahat.nat.tmpl where it won't get lost and move on. Beyond awful as interface design as grayed out text has such a specific meaning.
      • the attach templates call the upload handlers whether the attach or batch upload boxes are checked, passing different data. Off to look at TWiki::UI::Upload.
      • ok, TWiki::UI::Upload is far, far cleaner. Now I can see what's up with BatchUploadPlugin, too, which is that the author got started with an ugly hack trying to handle nested zip files and left it partly done. The approach is inherently problematic, though, as it introduces processing to every topic load for something that is rarely needed.
3. What this situation calls more is the right reuse - Archive::Extract rather than Archive::Zip, for one thing, proper integration with the existing Upload interface for another.

Solution

• In this case, I'm going to write a new plugin, rather than trying to salvage BatchUploadPlugin. Time to thoroughly re-factor. The history shows how we got to this place.