287109 - [SECURITY] Names of private products/components can be exposed on certain CGIs

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect, P1)

Description

[Frédéric Buclin]

*** Please do not remove the security flag till we have clearly identified if
there are "real" security holes ***

Per discussion with justdave in IRC, it appears that some validation functions
have confusing name or do incomplete validation.

Two examples:

1. In enter_bug.cgi, CanEnterProduct() is called to "check and make sure that
the user has permission to enter a bug against this product." (comment taken
from that file, line 293). Based on this statement, I removed an additional
check as part of bug 282267, because I thought CanEnterProduct() and
lsearch(\@::enterable_products, $product) == -1 were both doing the same job. It
appears that it is *not* the case!! So the validation done by CanEnterProduct()
is incomplete, as it should also check the 'disallownew' bit.

2. Independently of my "mistake" about this validation in enter_bug.cgi, it's
still possible to enter bugs in "closed" products by hacking the URL given to
post_bug.cgi as this file "only" calls CanEnterProduct()! I can reproduce this
on both 2.18 and 2.20 (I haven't tested with 2.16).


There are several functions (in globals.pl) with similar names: CanEnterProduct,
GetEnterableProducts and GetSelectableProducts for example. But they do not seem
to do what they really should do and some of them seem to be incorrectly used.
Coming back to my previous examples 1. and 2., CanEnterProduct() should consider
the 'disallownew' bit. There may be more...

Before releasing any new version, we should make sure that everything is ok...

Comment 1

[Max Kanat-Alexander]

CanEnterProduct should just use @::enterable_products, and should call
GetVersionTable to make sure that that variable exists.

Comment 2

[Frédéric Buclin]

(22:14:31) joel: Yes... if you want to get rid of the versioncache thing (die
versioncache!!), you need to have GetEnterableProducts take that into account
(22:15:04) LpSolit: joel, what about versioncache? I'm ok to remove it ;)
(22:15:25) kiko: yeah, it should be a goner by now :-(
(22:15:25) joel: yes... make it go away

Comment 3

[Gervase Markham [:gerv]]

If the situation is as you describe, it means that (if they know the names of
them) attackers can enter bugs in products they shouldn't be able to. While a
nuisance, this doesn't give them access to any confidential information, does it?

Gerv

Comment 4

[Frédéric Buclin]

(In reply to comment #3)
> nuisance, this doesn't give them access to any confidential information, does it?

You are correct... about post_bug.cgi. The security flag is set till we are sure
such functions are not misused in other files, such as show_bug.cgi.

Comment 5

[Joel Peshkin]

IIUC, the problem is post_bug is that a product could be "closed for bug entry,"
but still accessible and a user who could have entered a bug in that product had
it been open could still enter a bug.

That doesn't make it as a security problem in my book.

---------------

HOWEVER -- In show_bug.cgi, the history of comments IS AVAILABLE even if the bug
is not, so long as we are in the "long format" mode.  So, if you hack form data,
you CAN get the comments.  ValidateBugID() is only called for a single bug.

Try this...

Search for a bunch of bugs, some non-public.
Click on LONG FORMAT
see results
select one bug, view it,  and log out
back up to the LONG FORMAT listing and hit reload...

Comment 6

[Myk Melez [:myk] [@mykmelez]]

> this doesn't give them access to any confidential information, does it?

Component names might be confidential in commercial products.

Comment 7

[Frédéric Buclin]

Attachment: fix CanEnterProduct(), trunk, v1

Comment 8

[Joel Peshkin]

Comment on attachment 179986 [details] [diff] [review]
fix CanEnterProduct(), trunk, v1

r=joel by inspection
I'd like a 2xr from someone who both inspects and tests it.

Comment 9

[Dave Miller [:justdave]]

(In reply to comment #5)
> HOWEVER -- In show_bug.cgi, the history of comments IS AVAILABLE even if the
> bug is not, so long as we are in the "long format" mode.  So, if you hack
> form data, you CAN get the comments.

For the record since nobody actually mentioned it here, that was filed
separately as bug 287880 and has already been fixed.

Comment 10

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 179986 [details] [diff] [review]
fix CanEnterProduct(), trunk, v1

>-# This function determines if a user can enter bugs in the named
>-# product.
>+# This function determines if a user can submit new bugs
>+# in the named product.

Nit: submit new bugs in -> enter bugs into
Nit: while you're updating the comment, if -> whether or not


>+    my $allow_new_bugs =
>+        $dbh->selectrow_array("SELECT CASE WHEN disallownew = 0 THEN 1 ELSE 0 END
>+                               FROM products LEFT JOIN components
>+                               ON components.product_id = products.id
>+                               WHERE products.name = ?
>+                               AND components.product_id IS NOT NULL " .
>+                               $dbh->sql_limit(1),
>+                               undef, $productname);
>+
>+    return 0 unless $allow_new_bugs;

Note that DBI's documentation suggests exercising caution when using
selectrow_array in a scalar context.  It doesn't seem there's any reason to do
it in this case, so it'd be better to do:

  my ($allow_new_bugs) = $dbh->selectrow_array(...);

However, you could also do this more simply with an inner join and using scalar
context:

    my $allow_new_bugs =
	$dbh->selectrow_array("SELECT CASE WHEN disallownew = 0 THEN 1 ELSE 0
END
			       FROM products JOIN components
			       ON components.product_id = products.id
			       WHERE products.name = ? " .
			       $dbh->sql_limit(1),
			       undef, $productname);

Then, $allow_new_bugs would contain 1 if disallownew = 0 and there are any
components, 0 if disallownew = 1 and there are any components, and undef (which
equals 0) if there are no components.

In testing: without the patch, I can enter bugs into disabled products, but I
can't enter bugs into componentless products.  When I try to enter a bug into a
componentless product by hacking the URL to enter_bug.cgi, switching the
product parameter from a valid product to the componentless product, I get:

  Sorry; there needs to be at least one component for this product
  in order to create a new bug.  Create a new component.

When I try to do so by saving the enter_bug.cgi form locally and hacking it, I
get the following whether or not I specify a component:

  To file this bug, you must first choose a component. 
  If necessary, just guess.

With the patch, I can't enter bugs into either kind of product.  Whether I hack
the URL or the form, both kinds of products give me the error:

Sorry, either the product PRODUCTNAME does not exist, or you don't have the
required permissions to enter a bug against that product.

So the fix does prevent me from entering bugs into products I shouldn't be
entering them into.  But in the process it makes the error message for
componentless products worse.  We need to give users accurate information about
why they can't enter bugs into products instead of just telling them they don't
have permission to do so.

The message for disabled products should be:

  Sorry, entering bugs into product PRODUCTNAME has been disabled.

For componentless products, it's:

  Sorry, product PRODUCTNAME needs to have at least one component
  before you can enter bugs into it.  Create a new component.


>-    $query .= "WHERE products.name = " . SqlQuote($productname) . " " .
>+    $query .= "WHERE products.name = ? " .
>               $dbh->sql_limit(1);
>-    PushGlobalSQLState();
>-    SendSQL($query);
>-    my ($ret) = FetchSQLData();
>-    PopGlobalSQLState();
>-    return ($ret);
>+
>+    return $dbh->selectrow_array($query, undef, $productname);

Is this part of the fix for this bug or just serendipitous modernization?

Comment 11

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 179986 [details] [diff] [review]
fix CanEnterProduct(), trunk, v1

>-# This function determines if a user can enter bugs in the named
>-# product.
>+# This function determines if a user can submit new bugs
>+# in the named product.

Nit: submit new bugs in -> enter bugs into
Nit: while you're updating the comment, if -> whether or not


>+    my $allow_new_bugs =
>+        $dbh->selectrow_array("SELECT CASE WHEN disallownew = 0 THEN 1 ELSE 0 END
>+                               FROM products LEFT JOIN components
>+                               ON components.product_id = products.id
>+                               WHERE products.name = ?
>+                               AND components.product_id IS NOT NULL " .
>+                               $dbh->sql_limit(1),
>+                               undef, $productname);
>+
>+    return 0 unless $allow_new_bugs;

Note that DBI's documentation suggests exercising caution when using
selectrow_array in a scalar context.  It doesn't seem there's any reason to do
it in this case, so it'd be better to do:

  my ($allow_new_bugs) = $dbh->selectrow_array(...);

However, you could also do this more simply with an inner join and using scalar
context:

    my $allow_new_bugs =
	$dbh->selectrow_array("SELECT CASE WHEN disallownew = 0 THEN 1 ELSE 0
END
			       FROM products JOIN components
			       ON components.product_id = products.id
			       WHERE products.name = ? " .
			       $dbh->sql_limit(1),
			       undef, $productname);

Then, $allow_new_bugs would contain 1 if disallownew = 0 and there are any
components, 0 if disallownew = 1 and there are any components, and undef (which
equals 0) if there are no components.

In testing: without the patch, I can enter bugs into disabled products, but I
can't enter bugs into componentless products.  When I try to enter a bug into a
componentless product by hacking the URL to enter_bug.cgi, switching the
product parameter from a valid product to the componentless product, I get:

  Sorry; there needs to be at least one component for this product
  in order to create a new bug.  Create a new component.

When I try to do so by saving the enter_bug.cgi form locally and hacking it, I
get the following whether or not I specify a component:

  To file this bug, you must first choose a component. 
  If necessary, just guess.

With the patch, I can't enter bugs into either kind of product.  Whether I hack
the URL or the form, both kinds of products give me the error:

Sorry, either the product PRODUCTNAME does not exist, or you don't have the
required permissions to enter a bug against that product.

So the fix does prevent me from entering bugs into products I shouldn't be
entering them into.  But in the process it makes the error message for
componentless products worse.  We need to give users accurate information about
why they can't enter bugs into products instead of just telling them they don't
have permission to do so.

The message for disabled products should be:

  Sorry, entering bugs into product PRODUCTNAME has been disabled.

For componentless products, it's:

  Sorry, product PRODUCTNAME needs to have at least one component
  before you can enter bugs into it.  Create a new component.


>-    $query .= "WHERE products.name = " . SqlQuote($productname) . " " .
>+    $query .= "WHERE products.name = ? " .
>               $dbh->sql_limit(1);
>-    PushGlobalSQLState();
>-    SendSQL($query);
>-    my ($ret) = FetchSQLData();
>-    PopGlobalSQLState();
>-    return ($ret);
>+
>+    return $dbh->selectrow_array($query, undef, $productname);

Is this part of the fix for this bug or just serendipitous modernization?

Comment 12

[Frédéric Buclin]

(In reply to comment #11)

> So the fix does prevent me from entering bugs into products I shouldn't be
> entering them into.  But in the process it makes the error message for
> componentless products worse.  We need to give users accurate information

I disagree! We said several times the user should not know if a product does not
exist or if it has restrictions on who can enter bugs into. Applying this idea
to disabled products or products without components, I think we should not give
the exact reason why the user cannot access this product... or am I missing
something? Any opinion guys?



> >-    $query .= "WHERE products.name = " . SqlQuote($productname) . " " .
> >+    $query .= "WHERE products.name = ? " .
> >               $dbh->sql_limit(1);
> >-    PushGlobalSQLState();
> >-    SendSQL($query);
> >-    my ($ret) = FetchSQLData();
> >-    PopGlobalSQLState();
> >-    return ($ret);
> >+
> >+    return $dbh->selectrow_array($query, undef, $productname);
> 
> Is this part of the fix for this bug or just serendipitous modernization?
> 

Modernization only.

Comment 13

[Dave Miller [:justdave]]

Here's what I said on IRC when LpSolit asked me about this:

<justdave_> LpSolit: Myk is correct.
<justdave_> As long as the user would otherwise be able to see the product
<LpSolit> justdave_: so you want to display some "confidential" information?
<justdave_> they should get an accurate reason why they can't enter in it.
<justdave_> neither of those are confidential situations
<justdave_> he's talking about if the user can already see the product, but the
product has been disabled or hasn't had components created yet.
<justdave_> that's completely different from if the product doesn't exist of if
the user doesn't have access to the product.
<justdave_> A user not having access to a product should be treated as if the
product doesn't exist
<justdave_> (or give an ambiguous message indicating that either is possible)
<justdave_> "doesn't exist or you don't have access to it"
<justdave_> if the user DOES have access to the product, but the product has
been closed for entry or there are not components defined in it, then there's no
reason to hide that from the user, as they can already see it anyway.
<justdave_> whether the user has access to the product should be checked prior
to checking for closed or no components though.

Comment 14

[Frédéric Buclin]

What you ask me to do is a pain. CanEnterProduct() has to reject a request
returning an appropriate value based on the following cases:

1) the user cannot enter the product
2) the product is closed for new bug entry
3) the product has no component

Most developers use "if (CanEnterProduct($product))" so that we should be
careful on the value returned by this function. I see only two possible values:
0 and undef, meaning that I cannot distinguish more than two cases. :(

Another possibility is to use an intermediate function CanEnterProductOrWarn()
which would return an appropriate error message based on the value returned by
CanEnterProduct(), assuming it can returned any value, for example -1 for no
access, 0 for a closed product and undef for a product with no component. But
then, we would need to change the logic in the whole code, replacing "if
(CanEnterProduct($product))" by "if (CanEnterProduct($product)) > 0". Is that a
safe solution? I'm not sure!

Any better idea?

Comment 15

[Frédéric Buclin]

Attachment: fix CanEnterProduct(), trunk, v2

I created a new CanEnterProductOrWarn() function which takes care of the value
returned by CanEnterProduct() in order to display the correct error message, if
any.

Comment 16

[Frédéric Buclin]

Comment on attachment 181728 [details] [diff] [review]
fix CanEnterProduct(), trunk, v2

Is my superreviewer satisfied now? :)

Comment 17

[Joel Peshkin]

Comment on attachment 181728 [details] [diff] [review]
fix CanEnterProduct(), trunk, v2


> 
>+  [% ELSIF error == "product_disabled" %]
>+    [% title = BLOCK %]Product closed for [% terms.Bug %] Entry[% END %]
>+    The product <em>[% product FILTER html %]</em> accepts no new [% terms.bug %].
>+

Make that bugs instead of bug and r=joel

Comment 18

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 181728 [details] [diff] [review]
fix CanEnterProduct(), trunk, v2

>+  [% ELSIF error == "product_disabled" %]
>+    [% title = BLOCK %]Product closed for [% terms.Bug %] Entry[% END %]
>+    The product <em>[% product FILTER html %]</em> accepts no new [% terms.bug %].

Nit: this should be "Sorry, entering bugs into product [product name] has been
disabled."

Otherwise this looks good and works as expected.  r=myk

Comment 19

[Myk Melez [:myk] [@mykmelez]]

Requesting approval but not setting it, since this will get approved for checkin
right before release of the security update.

Comment 20

[Frédéric Buclin]

Attachment: final fix for the trunk (error message updated), v2.1

Fixing myk's nit about the error message. Forwarding r+

Comment 21

[Frédéric Buclin]

Attachment: 2.18 backport, v1

In 2.18, CanEnterProductOrWarn() requires an additional parameter
$unlock_tables because process_bug.cgi calls this function while most tables
are locked. This parameter is not required in 2.19 because tables are
automatically unlocked when Throw*Error() is called, see bug 277782.

Comment 22

[Frédéric Buclin]

Comment on attachment 182051 [details] [diff] [review]
2.18 backport, v1

I'm not sure if two reviews are required for this backport. If not, the first
who reviews this patch should feel free to remove the other request for review.

Comment 23

[Frédéric Buclin]

Attachment: 2.16 backport, v1

This patch does not alter process_bug.cgi. I'm not sure it worths fixing it.
And reports.cgi seems not to be affected.

Comment 24

[Frédéric Buclin]

Comment on attachment 182062 [details] [diff] [review]
2.16 backport, v1

Nick has an active 2.16.8 installation. I haven't one so please test it
carefully! ;)

Comment 25

[Frédéric Buclin]

Attachment: 2.16 backport, v1.1

oops, I forgot a 'exit;' after a call to DisplayError(). Sorry for bug spam.

Comment 26

[Frédéric Buclin]

It's really a pain!!! When moving a bug to a different product, Bugzilla asks
you to choose the component, the version and the target_milestone independently
whether or not the product has group restrictions or is closed for bugs entry!!!
Even with my patch applied! :(

Comment 27

[Nick Barnes]

Attachment: 2.16 backport, v1.2

2.16 backport patch v1,2. like v1.1 except it tests in process_bug.cgi that a
bug is not being moved to a product which is closed for bug entry.  The
placement of the test here is tricky.  We don't want to leak version and
component names of a product to a user who is not entitled to see that product.
 So we test *before* showing the version/component selection dropdowns.
However, we test by calling CanEnterProductOrWarn().  What if usebuggroupsentry
is not set but usebuggroups is set, and the user isn't in the necessary group? 
Should such a user be allowed to move bugs into the product? What's the desired
semantics?

Comment 28

[Nick Barnes]

Attachment: 2.16 backport, v1.3

Let's try again; LpSolit spotted a problem straight off, and I think I've
learned how to use cvs diff as well.

Comment 29

[Frédéric Buclin]

Comment on attachment 182051 [details] [diff] [review]
2.18 backport, v1

I will update this patch soon...

Comment 30

[Frédéric Buclin]

Comment on attachment 182035 [details] [diff] [review]
final fix for the trunk (error message updated), v2.1

There is still an issue in process_bug.cgi...

Comment 31

[Frédéric Buclin]

Comment on attachment 182035 [details] [diff] [review]
final fix for the trunk (error message updated), v2.1

There is still an issue in process_bug.cgi...

Comment 32

[Frédéric Buclin]

Attachment: patch for the trunk, v3

Fixes leakage in process_bug.cgi.

Comment 33

[Nick Barnes]

I'm having some difficulty testing this on 2.16.8 as I don't have a concrete
list of use cases and the desired outcome, for all the affected code.  This v1.3
patch seems to work for the tests which I have devised so far.

Comment 34

[Frédéric Buclin]

(In reply to comment #33)
> I'm having some difficulty testing this on 2.16.8 as I don't have a concrete
> list of use cases and the desired outcome, for all the affected code.  This v1.3
> patch seems to work for the tests which I have devised so far.

Your version 1.3 of the 2.16 backport does not consider the fact that all bugs
could already be in the product we are moving to and you would then generate a
useless (and incorrect) error message. I'm waiting for joel's and myk's review
of the v3 patch before backporting it to 2.18 and 2.16.

Comment 35

[Joel Peshkin]

Comment on attachment 182124 [details] [diff] [review]
patch for the trunk, v3

r=joel by inspection if you've tested it.  
2xr still needed since it touches enough security stuff.

Comment 36

[Nick Barnes]

(In reply to comment #34)
> Your version 1.3 of the 2.16 backport does not consider the fact that all bugs
> could already be in the product we are moving to and you would then generate a
> useless (and incorrect) error message. I'm waiting for joel's and myk's review
> of the v3 patch before backporting it to 2.18 and 2.16.

Well, it does consider that fact.  It documents in the comment that it doesn't
handle it well.
The only alternative is to iterate through the bug IDs getting their current
products, before potentially asking CanEnterProductOrWarn.  I'm happy to write
that code, if we decide it's indicated.  But it'll make this a bigger patch, and
we should endeavour to keep it small for a security fix to a maintenance branch.

Comment 37

[Frédéric Buclin]

(In reply to comment #36)
> Well, it does consider that fact.  It documents in the comment that it doesn't
> handle it well.

So it's a bad solution. ;)


> The only alternative is to iterate through the bug IDs getting their current
> products, before potentially asking CanEnterProductOrWarn.

Huh? Please read my last patch first. I already fixed that problem.


  I'm happy to write
> that code, if we decide it's indicated.

There is no need to be two developers on this bug. This one is already assigned
to me and I feel able to fix it without any help. But thanks for proposing your
help! You can help by reviewing the 2.16 patch as soon as I upload a new one. :)

Comment 38

[Frédéric Buclin]

Comment on attachment 182124 [details] [diff] [review]
patch for the trunk, v3

/me hopes this bug to be fixed before 2007.

Comment 39

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 182124 [details] [diff] [review]
patch for the trunk, v3

>+    $cgi->param('component_id', $comp_id);

Nice simplification of the code!

I haven't tested this, but it looks good. r=myk

Comment 40

[Frédéric Buclin]

Attachment: 2.18 backport, v2

Comment 41

[Frédéric Buclin]

Attachment: 2.16 backport, v2

I have no 2.16 installation, so I cannot test this patch, except running
runtests.pl, which does not complain. Table fields have changed a lot between
2.16 and 2.19 so I may be wrong here. This patch *really* needs to be tested.
Nick, could you do some tests for 2.16? I just realized that you did not test
previous 2.16 patches, even yours, because there are invalid field names in
them and any trivial test would have failed immediately. ;)

Comment 42

[Max Kanat-Alexander]

I made a 2.16 testing installation for this bug at:

http://landfill.bugzillla.org/bz287109/

Comment 43

[Max Kanat-Alexander]

Typo correction: http://landfill.bugzilla.org/bz287109/

Comment 44

[Myk Melez [:myk] [@mykmelez]]

Attachment: test results for 2.18 and 2.16 v2 patches

A basic set of tests for these fixes are to create componentless, disabled, and
confidential products and then try to load enter_bug.cgi and post_bug.cgi,
specifying those products, both before and after applying the patch.

I ran such tests on 2.18 and 2.16 installations and got the attached results.

Comment 45

[Myk Melez [:myk] [@mykmelez]]

Attachment: updated test results for 2.18 and 2.16 patches

Here are updated test results after I enabled "usebuggroupsentry" in the 2.16
installation's parameters.

Comment 46

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 182735 [details] [diff] [review]
2.18 backport, v2

>Index: globals.pl

>-# This function determines if a user can enter bugs in the named
>-# product.
>+# This function determines whether or not a user can enter
>+# bugs into the named product.

Nit: writing the comment this way is a little easier to read:

>+# This function determines whether or not a user can enter bugs
>+# into the named product.

(Putting the line break between the phrases "a user can enter bugs" and "into
the named product" makes the comment easier to read than putting it within the
phrase "can enter bugs".)


>+    my $allow_new_bugs =
>+        $dbh->selectrow_array("SELECT CASE WHEN disallownew = 0 THEN 1 ELSE 0 END

Nit: I recommend "my ($allow_new_bugs) = ...".


>Index: process_bug.cgi

>+    my $check_can_enter =
>+        $dbh->selectrow_array("SELECT 1 FROM bugs

Nit: better as "my ($check_can_enter) = ...".


Nit: the message for componentless products should be "Sorry, the product
<product> has to have at least one component in order for you to enter a bug
into it.  Please contact <maintainer> and ask them to add a component to this
product."

Nit: the message for disabled products should be "Sorry, entering bugs into the
product <product> has been disabled." (adding the word "the"--yes, I realize I
missed this on the trunk review)

Nit: the message for confidential products should be "Sorry, either the product
<product> does not exist or you aren't authorized to enter a bug into that
product."

All comments nits; this looks good, and tests show that it works well. r=myk

Comment 47

[Myk Melez [:myk] [@mykmelez]]

> Nit: the message for confidential products should be "Sorry, either the product
> <product> does not exist or you aren't authorized to enter a bug into that
> product."

Err, sorry, revising this nit.  The message should say, "Sorry, either the
product <product> does not exist or you aren't authorized to enter a bug into it."

Comment 48

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 182741 [details] [diff] [review]
2.16 backport, v2

>Index: globals.pl

>+sub CanEnterProductOrWarn {

Seems like this works, and 2.16 is virtually EOLed anyway, so it probably
doesn't matter, but is there a particular reason you combined the work done by
both CanEnterProduct and CanEnterProductOrWarn in other patches into a single
function in this patch?  It seems like unnecessarily divergent code paths.


>+    SendSQL("SELECT CASE WHEN disallownew = 0 THEN 1 ELSE 0 END
>+             FROM products INNER JOIN components
>+             ON components.program = products.product
>+             WHERE products.product = " . SqlQuote($productname) . " LIMIT 1");
>+
>+    my $status = FetchOneColumn();

Nit: though I don't think it'll cause problems in this particular case, queries
in subroutines that don't push and pop the global SQL state run the risk of
corrupting a query done outside the subroutine whose data has not been fully
retrieved when the subroutine gets called.  You should push and pop the global
SQL state around these statements.

Message nits from the 2.18 patch apply here as well.

Otherwise, the code looks good and the tests all come up smelling like roses.
r=myk

Comment 49

[Frédéric Buclin]

Thanks a lot myk for your reviews and tests! :)

Comment 50

[Dave Miller [:justdave]]

ok, release is imminent, let's roll :)

Comment 51

[Max Kanat-Alexander]

Unfortunately, the patch author isn't around to do these checkins, and I don't
feel comfortable addressing the message nits on my personal checkin. Perhaps we
should have another bug marked as "trivial" for dealing with them.

Tip:

Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.114; previous revision: 1.113
done
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.323; previous revision: 1.322
done
Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.116; previous revision: 1.115
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.255; previous revision: 1.254
done
Checking in reports.cgi;
/cvsroot/mozilla/webtools/bugzilla/reports.cgi,v  <--  reports.cgi
new revision: 1.74; previous revision: 1.73
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v
 <--  user-error.html.tmpl
new revision: 1.108; previous revision: 1.107
done


2.18:
Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.94.2.3; previous revision: 1.94.2.2
done
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.270.2.9; previous revision: 1.270.2.8
done
Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.88.2.7; previous revision: 1.88.2.6
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.205.2.19; previous revision: 1.205.2.18
done
Checking in reports.cgi;
/cvsroot/mozilla/webtools/bugzilla/reports.cgi,v  <--  reports.cgi
new revision: 1.72.2.2; previous revision: 1.72.2.1
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v
 <--  user-error.html.tmpl
new revision: 1.61.2.11; previous revision: 1.61.2.10
done


2.16:

Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.66.2.4; previous revision: 1.66.2.3
done
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.169.2.26; previous revision: 1.169.2.25
done
Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.52.2.9; previous revision: 1.52.2.8
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.125.2.13; previous revision: 1.125.2.12
done

Comment 52

[Frédéric Buclin]

(In reply to comment #51)
> Unfortunately, the patch author isn't around to do these checkins, and I don't
> feel comfortable addressing the message nits on my personal checkin. Perhaps we
> should have another bug marked as "trivial" for dealing with them.

mkanat, yes. That's what I had in mind. Don't worry for that. :)

Comment 53

[Alexis Sukrieh]

Comment on attachment 182741 [details] [diff] [review]
2.16 backport, v2

This really seems to break something in bugzilla 2.16. enter_bug.cgi now gives
me always an error saying that I'm not alowed to access the component while I
do have a component.

Comment 54

[Max Kanat-Alexander]

Comment on attachment 182741 [details] [diff] [review]
2.16 backport, v2

This should actually still be r+, since it passed review, and the review was
myk's. I talked to sukria about it on IRC.

If we do find a problem with this patch... we'll probably have to open anohther
bug and pull 2.16.9 if possible.

Comment 55

[Alexis Sukrieh]

Just for the record, I confirm that the bug still appears with a 2.16.9 tarball.
The problem really seems to come from the fact that I use a database wich was
created for a 2.16.7 system. Maybe something is missing somewhere... (running
checksetup.pl did not help)