code/ causes release-notes.html to fail




8 years ago
8 years ago


(Reporter: michaelc, Unassigned)



(Whiteboard: [3.4 only])


(1 attachment)



8 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_4; en-us) AppleWebKit/533.16 (KHTML, like Gecko) Version/5.0 Safari/533.16
Build Identifier: 

In Bugzilla 3.4.x, the _get_extension_requirements method includes the following:
    my @extensions = glob(bz_locations()->{'extensionsdir'} . "/*");
    foreach my $extension (@extensions) {
        my $file = "$extension/code/";

The glob results in a tainted @extensions array.  If "code/" is present, the subsequent "rdo" will fail if called in taint mode.

"page.cgi?id=release-notes.html" runs in taint mode and calls utilizes this function to generate module requirement lists.  If code/ is present it will die non-gracefully.

To fix, glob could be replaced with readdir or the resulting output can be untainted:

diff -u -r1.60.2.6
---	11 Sep 2009 16:57:28 -0000
+++	30 Jun 2010 19:47:51 -0000
@@ -295,6 +295,7 @@
     my @extensions = glob(bz_locations()->{'extensionsdir'} . "/*");
     foreach my $extension (@extensions) {
         my $file = "$extension/code/";
+        ( $file ) = $file =~ /(.*)/;
         if (-e $file) {
             my $safe = new Safe;
             # This is a very liberal Safe.

Reproducible: Always

Steps to Reproduce:
1.  Install Bugzilla 3.4.x
2.  echo "1;" > extensions/example/code/
3.  Go to page.cgi?id=release-notes.html
Actual Results:  
undef error - Can't locate object method "splitpath" via package "File::Spec" at /usr/lib/perl5/5.8.8/CGI/ line 359.

The error message is misleading because CGI::Carp is failing and the original $@ is not being passed along.

Comment 1

8 years ago
  Oh, that sounds like a problem indeed. However, 3.4 is locked to security fixes, and its extension system is most definitely deprecated in favor of Bugzilla 3.6's Extension system. So it is unlikely that this will be fixed. However, if you want to supply a patch, it is indeed just a one-line change, so we could probably take it in case there is a 3.4.8 release in the future.
Severity: normal → minor
Ever confirmed: true
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [3.4 only]
Target Milestone: --- → Bugzilla 3.4

Comment 2

8 years ago
  Why isn't this failing on, though? I wrote an extension there that has install-requirements, and they are running 3.4.

Comment 3

8 years ago
Created attachment 455273 [details] [diff] [review]
untaints filename before rdo

Comment 4

8 years ago
Thanks! You might want to ask me for review so that I don't forget. Here's info on our development process:

Comment 5

8 years ago
(In reply to comment #2)
>   Why isn't this failing on, though? I wrote an extension
> there that has install-requirements, and they are running 3.4.

I am trying to see why it would not be failing on other systems.  I asked ghendricks at #testopia, and he says it is working for him, so perhaps it is something local to me.

From a theoretical standpoint, it seems like it should fail.  The File::Glob docs say that the output should be expected to be tainted, and Safe->rdo should be subject to taint checks.

Comment 6

8 years ago
Interesting point to note that though it does not fail for me, it does not include the extension required or optional modules for Testopia, even though it appears to be getting them dynamically. It is arguable whether they should be included in the release-notes, but that is the only thing I see wrong here.

Comment 7

8 years ago
Perhaps in the landfill install and on, something is preventing the glob from seeing the extensions at all when running from the web server (and therefore not running the offending block at all).  We run suexec as the user which owns the Bugzilla directory.

After applying patch, our copy does show the additional Testopia requirements (JSON, Text::Diff, etc.):

It is not that we desire to see the additional requirements on this page so much as avoiding a fatal error if it does detect the extensions.

Comment 8

8 years ago
You know, this could somehow just be related to suexec, in some way that I don't understand. We never supported suexec until 3.6, and I know that there were problems surrounding extensions with suexec that will probably be there until 4.0.

Comment 9

8 years ago
Ultimately, I'm going to have to say that this is WONTFIX, since the 3.4 branch is locked to sec fixes only, and we can't reproduce the problem locally.
Last Resolved: 8 years ago
Resolution: --- → WONTFIX


8 years ago
Target Milestone: Bugzilla 3.4 → ---
You need to log in before you can comment on or make changes to this bug.