Its possible for the bugzilla group security stuff to be bypassed by filing a duplicate bug. This will add you to the cc list, and so, by default, you'll be able to see the bug. To reproduce: 1. Have a restricted bug which is open to people on the cclist (http://landfill.tequilarista.org/bugzilla-tip/show_bug.cgi?id=278) 2. As a user not in that group, file a bug, and dupe it against the bug which you cannot see (http://landfill.tequilarista.org/bugzilla-tip/show_bug.cgi?id=279) 3. You can now view bug 278

I don't know if bug 278 is open for everyone but I can see that bug. (foodreplicator....) I'm not logged in but I can see this bug with NS4.7x and Mozilla but not with IE (I never used it on bugzilla-> no bugzilla cookies). With IE I get ".exp problem ....) Maybe I can see this bug because I'm watching Bradley ?

Um, I could see that bug when I wasn't logged in. but I couldn't when logged in as another user (matti, its a sepatate db, so tyou need a separate account) This is, like, bad.

OK, what we need to do here is: a) you can't dupe a bug against a bug you can't see. b) if someone else dupes the bug, if the reporter can't see the new bug, the CC should not be added. For case B it probably wouldn't hurt to put up an intermediate screen warning the user doing the duping that the reporter can't see it.

The "I can see bugs if I'm not logged in" bug has been filed as bug 96099. dave: That sounds fine.

Not sure how well that patch works, although I'm pretty sure it gets the job done. My database is down right now so I can't test it. Maybe someone else can? :)

Using ValidateBugID() negates the need for the following block: 707 SendSQL("SELECT bug_id FROM bugs WHERE bug_id = " . SqlQuote($num)); 708 $num = FetchOneColumn(); 709 if (!$num) { 710 PuntTryAgain("You must specify a valid bug number of which this bug " . 711 "is a duplicate. The bug has not been changed.") 712 } Of course, this patch doesn't do anything towards part b) [and its sub-part].

I know, Jake. I didn't have time to work on part B last night. I mentioned it was only for part A. I've got some more time now so I'll take this bug.... I'll remove the part you mentioned in the patch and post that later with the new patch.

Okay, I'm new to accepting bugs. Guess I've got to change the owner first and then accept it.. (sorry for spam).

Really taking bug now...

I have a somewhat functional patch with a few small glitches in it that I need to work out. I'm posting it for some general feedback and ideas. 1) $::FORM{'knob'} gets set to numeric value '2' in the code somewhere. I need to figure out why it does that. At that point however, it should always be set to "duplicate" so for the time being, I hard coded it in there with a print statement displaying the value of it. 2) If both the reporter and the duper don't have access to the bug, the header displays twice. I'll have a fix for that shortly but I've been spending most of my time trying to debug problem 1. I'm a bit new to the Bugzilla code so I'm sure I've overlooked some stuff but any help/feedback are definitely welcome. In any case, if you can see a bug and are duping a bug with a reporter that can't, you get to choose whether to add them to CC or not. Or throw away changes. This is a start to fixing bug 92078. Attachment on the way...

Adding justdave to cc (guess he got removed when I grabbed the bug)

The patch I just uploaded looks like it will fix everything. Initially I went about it the wrong way. I realized that when I studied the bugzilla code some more. Dave just applied the patch on http://landfill.tequilarista.org/bz96085/ On IRC, Dave OK'd the CGI.pl part of my patch with the optimization since I am using similar code in process_bug.cgi

Small oversight in my latest patch. if (defined $::FORM{'dup_id'}) { should be: if (defined $::FORM{'dup_id'} && $::FORM{'knob'} == "duplicate") {

In DuplicateUserConfirm() you have: + # Make sure the bug exists in the database. + if (!MoreSQLData()) { + DisplayError("Bug #$original does not exist.") && exit; + } Which really isn't needed because you already called: + ValidateBugID($::FORM{'dup_id'}); Which made sure both the $::FORM{'dup_id'} was numeric and exits in the database. I guess it really doesn't hurt anything because it's not another database call, but just thought it'd be worth mentioning :) I'm also not fond of the relying on the words "Do NOT" from the confirm page. I almost think that be better as option buttons: Add the user to the CC list so they can see the bug?: ( ) Yes ( ) No [Commit] The text should also, IMHO, tell the person who is doing the duping whether adding the user to the CC list will allow them to immediately see the bug (cclist_accessible == 1) or just if it's just a possiblity down the line that someone will turn on the CC list accessibility.

The initial scan of attachment 47026 [details] [diff] [review] looks good (I've only looked at the code, I haven't tested anything). I think this will probably have a conflict with bug 92266 (not a code conflict, but the changes in the two patches are close enough together that CVS will probably flag it... it's easy enough to fix, though, seeing how there isn't a code conflict).

Attachment 47052 [details] [diff] (1.5) is the same as 47026 (1.4) except that I have added checked="checked" for the "NO" radio button. Otherwise, yes was the default and IMO, users should have to say yes as opposed to having to say no.

Moving to new product Bugzilla...

Attachment 47052 [details] [diff] works for me. r=jake Checked In.

*** Bug 93508 has been marked as a duplicate of this bug. ***