process_bug.cgi doesn't check viewing permissions

RESOLVED FIXED in Bugzilla 2.14

Status

()

Bugzilla
Bugzilla-General
P3
critical
RESOLVED FIXED
17 years ago
4 years ago

People

(Reporter: Jesse Ruderman, Assigned: myk)

Tracking

unspecified
Bugzilla 2.14
Other
Other

Details

(Whiteboard: security)

Attachments

(6 attachments)

(Reporter)

Description

17 years ago
Steps to reproduce:
1. Find a restricted bug (bug 28698, for example).
2. Save the page you're reading right now (a normal show_bug.cgi page).
3. Change the form action to an absolute URL on the saved page.
4. Modify hidden form elements so they read:
<INPUT TYPE=HIDDEN NAME="delta_ts" VALUE="19950000000000">
<INPUT TYPE=HIDDEN NAME="longdesclength" VALUE="0">
<INPUT TYPE=HIDDEN NAME="id" VALUE=28698>     (use bug number from step 1)
5. Load the modified page.
6. Click "commit".

Actual result:
All of the comments on bug 28698 are displayed.

Expected result:
process_bug.cgi says something to the effect of "You don't have permission to 
view this bug, so you're not allowed to change it either."

Notes:
No "mid-air collision" items are listed.  This probably has something to do 
with the bogus timestamp.
(Reporter)

Comment 1

17 years ago
Is anyone working on this set of bugs?
(Reporter)

Updated

17 years ago
Blocks: 66091
Whiteboard: 2.14

Comment 2

16 years ago
Created attachment 25673 [details]
Hidden Bug Viewer Tool

Comment 3

16 years ago
This bug makes bug groups for security purposes pretty useless, in all bugzilla
installations. Upping severity. Maybe there's a simple fix?
Severity: normal → critical

Comment 4

16 years ago
If you have editbugs permissions, you'll see all comments immediately.
If you are logged in as someone who doesn't have editbugs priviledges, you get a
message that you are not allowed to change the component from "Security: Crypto"
to "Tracking". By modifying the form and iterating over all fields it may be
possible to obtain all settings for the bug. I haven't tried it though.

Updated

16 years ago
Whiteboard: 2.14 → 2.14,security
(Reporter)

Comment 5

16 years ago
Blake says it's possible to use this to modify a restricted bug, for example to 
change the groupset and make it no longer be restricted.
*** Bug 69913 has been marked as a duplicate of this bug. ***
Bug 61193 is what Jesse just said Blake said.  There's another version of the 
"hacker's tool" attached to bug 61193 with description of how to actually change 
things in the bug.  I'm making it a dupe of this one because it's the same 
problem at the core.  When one is fixed, the other will be also.
I got the impression from the other bug you could change the bug number with
that tool.  Is that correct?
moving to real milestones...
Whiteboard: 2.14,security → security
Target Milestone: --- → Bugzilla 2.14

Comment 10

16 years ago
Matthew: You can't change the bug number, but you can change any other field.  
See the attachment to that bug; enter the number of the bug you want to change 
at the topic, modify any field, and commit.
(Assignee)

Comment 11

16 years ago
I'm working on some code to fix this, so reassigning the bug to myself.
Assignee: tara → myk
(Assignee)

Comment 12

16 years ago
Created attachment 36407 [details] [diff] [review]
patch to validate bug IDs and authorize users to modify them

Comment 13

16 years ago
My personal preference for the routine:
    if ($i =~ /^id_/) {
      push @idlist, substr($i, 3);
    }

would be:
    if ($i =~ /^id_(\d+)/) {
        push @idlist, $1;
    }

----

  ($groupset & $::usergroupset) == $groupset

There's a comment somewhere in bugzilla that groupset is 64-bit and the
submitter isn't sure if perl handles 64 bit math (I'm also not sure either way,
but thought it worth mentioning).

-----

If perl is capable of the math, then I'll try to test this patch tomarrow.

Updated

16 years ago
Keywords: patch, review
(Assignee)

Comment 14

16 years ago
Created attachment 36437 [details] [diff] [review]
patch that abstracts out bug id validation
(Assignee)

Comment 15

16 years ago
Perl can handle integers of arbitrary size using its Math::BigInt, which appears
to have been part of the default install since at least 5.004, so we could
either use that or do it in a query.  I will investigate further.
Status: NEW → ASSIGNED
(Assignee)

Comment 16

16 years ago
Created attachment 36558 [details] [diff] [review]
uses MySQL for bitset operations
(Assignee)

Comment 17

16 years ago
Created attachment 36578 [details] [diff] [review]
fixes typo in previous patch
(Assignee)

Comment 18

16 years ago
Created attachment 36582 [details] [diff] [review]
takes comments in bug 39527 into account
(Assignee)

Comment 19

16 years ago
Oops, I meant "takes comments in bug 39526 into account".  Hopefully the latest
patch is the last.  It contains a slight variation of Jake's preferred regex and
doesn't use the unnecessary GroupExists and UserInGroup functions to check
whether or not the user is authorized to access the bug if "usebuggroups" is
turned on (since checking $::usergroupset against the groupset of the bug
accomplishes the same goal).

Comment 20

16 years ago
Checked in.
r=jake
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Moving to Bugzilla product
Component: Bugzilla → Bugzilla-General
Product: Webtools → Bugzilla
Version: other → unspecified
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.