Last Comment Bug 39524 - process_bug.cgi doesn't check viewing permissions
: process_bug.cgi doesn't check viewing permissions
Status: RESOLVED FIXED
security
:
Product: Bugzilla
Classification: Server Software
Component: Bugzilla-General (show other bugs)
: unspecified
: Other Other
: P3 critical (vote)
: Bugzilla 2.14
Assigned To: Myk Melez [:myk] [@mykmelez]
: default-qa
Mentors:
: 69913 (view as bug list)
Depends on:
Blocks: 66091
  Show dependency treegraph
 
Reported: 2000-05-16 16:58 PDT by Jesse Ruderman
Modified: 2012-12-18 20:46 PST (History)
5 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Hidden Bug Viewer Tool (1.36 KB, text/html)
2001-02-19 18:29 PST, Andreas Franke (gone)
no flags Details
patch to validate bug IDs and authorize users to modify them (4.91 KB, patch)
2001-05-29 14:41 PDT, Myk Melez [:myk] [@mykmelez]
no flags Details | Diff | Splinter Review
patch that abstracts out bug id validation (5.71 KB, patch)
2001-05-29 16:15 PDT, Myk Melez [:myk] [@mykmelez]
no flags Details | Diff | Splinter Review
uses MySQL for bitset operations (6.62 KB, patch)
2001-05-30 15:52 PDT, Myk Melez [:myk] [@mykmelez]
no flags Details | Diff | Splinter Review
fixes typo in previous patch (6.62 KB, patch)
2001-05-30 16:41 PDT, Myk Melez [:myk] [@mykmelez]
no flags Details | Diff | Splinter Review
takes comments in bug 39527 into account (6.13 KB, patch)
2001-05-30 16:51 PDT, Myk Melez [:myk] [@mykmelez]
no flags Details | Diff | Splinter Review

Description Jesse Ruderman 2000-05-16 16:58:19 PDT
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.
Comment 1 Jesse Ruderman 2000-10-27 16:55:00 PDT
Is anyone working on this set of bugs?
Comment 2 Andreas Franke (gone) 2001-02-19 18:29:36 PST
Created attachment 25673 [details]
Hidden Bug Viewer Tool
Comment 3 Andreas Franke (gone) 2001-02-19 18:35:44 PST
This bug makes bug groups for security purposes pretty useless, in all bugzilla
installations. Upping severity. Maybe there's a simple fix?
Comment 4 Andreas Franke (gone) 2001-02-19 18:53:41 PST
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.
Comment 5 Jesse Ruderman 2001-02-22 19:05:54 PST
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.
Comment 6 Dave Miller [:justdave] (justdave@bugzilla.org) 2001-02-22 19:24:36 PST
*** Bug 69913 has been marked as a duplicate of this bug. ***
Comment 7 Dave Miller [:justdave] (justdave@bugzilla.org) 2001-02-22 19:34:09 PST
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.
Comment 8 Matthew Tuck [:CodeMachine] 2001-02-22 21:37:14 PST
I got the impression from the other bug you could change the bug number with
that tool.  Is that correct?
Comment 9 Dave Miller [:justdave] (justdave@bugzilla.org) 2001-02-27 19:13:02 PST
moving to real milestones...
Comment 10 Blake Ross 2001-03-11 20:42:35 PST
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.
Comment 11 Myk Melez [:myk] [@mykmelez] 2001-05-18 11:19:04 PDT
I'm working on some code to fix this, so reassigning the bug to myself.
Comment 12 Myk Melez [:myk] [@mykmelez] 2001-05-29 14:41:50 PDT
Created attachment 36407 [details] [diff] [review]
patch to validate bug IDs and authorize users to modify them
Comment 13 Jacob Steenhagen 2001-05-29 15:25:35 PDT
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.

Comment 14 Myk Melez [:myk] [@mykmelez] 2001-05-29 16:15:03 PDT
Created attachment 36437 [details] [diff] [review]
patch that abstracts out bug id validation
Comment 15 Myk Melez [:myk] [@mykmelez] 2001-05-30 10:50:24 PDT
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.
Comment 16 Myk Melez [:myk] [@mykmelez] 2001-05-30 15:52:34 PDT
Created attachment 36558 [details] [diff] [review]
uses MySQL for bitset operations
Comment 17 Myk Melez [:myk] [@mykmelez] 2001-05-30 16:41:30 PDT
Created attachment 36578 [details] [diff] [review]
fixes typo in previous patch
Comment 18 Myk Melez [:myk] [@mykmelez] 2001-05-30 16:51:46 PDT
Created attachment 36582 [details] [diff] [review]
takes comments in bug 39527 into account
Comment 19 Myk Melez [:myk] [@mykmelez] 2001-05-30 16:55:41 PDT
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 Jacob Steenhagen 2001-05-31 08:55:20 PDT
Checked in.
r=jake
Comment 21 Dave Miller [:justdave] (justdave@bugzilla.org) 2001-09-02 23:38:07 PDT
Moving to Bugzilla product

Note You need to log in before you can comment on or make changes to this bug.