Last Comment Bug 54556 - sanitycheck.cgi can be run by unprivileged accounts
: sanitycheck.cgi can be run by unprivileged accounts
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: Bugzilla-General (show other bugs)
: unspecified
: All All
: P3 minor (vote)
: Bugzilla 2.14
Assigned To: Tara Hernandez
: default-qa
Mentors:
http://bugzilla.mozilla.org/sanityche...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2000-09-28 14:35 PDT by Adam Spiers
Modified: 2012-12-18 20:46 PST (History)
3 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
restricts sanitycheck.pl to users with "editbugs" privileges (1.12 KB, patch)
2001-05-31 15:38 PDT, Myk Melez [:myk] [@mykmelez]
no flags Details | Diff | Splinter Review
breaks out confirm_login (1.13 KB, patch)
2001-06-01 17:53 PDT, Myk Melez [:myk] [@mykmelez]
no flags Details | Diff | Splinter Review

Description Adam Spiers 2000-09-28 14:35:25 PDT
http://bugzilla.mozilla.org/sanitycheck.cgi can be run from an
ordinary bugzilla account (e.g. mine).  On a bugs database
as large as bugzilla.mozilla.org, that effectively counts as
a DoS if done repeatedly.
Comment 1 Dave Miller [:justdave] (justdave@bugzilla.org) 2000-09-28 23:25:04 PDT
CCing a couple mozilla.org folks.  Any comments on this?
Comment 2 Dan Mosedale (:dmose) 2000-09-29 11:40:42 PDT
He's got a point.  Making it require a logged-in, priviledged account to use
wouldn't be a bad thing.  Another reason for doing so is that it makes public
some weird inconsistencies in the database, and it's not inconceivable that
these could somehow be used in a security exploit.

It is worth keeping in mind that it's essentially impossible to prevent DoS
attacks on apps like Bugzilla anyway.  All you have to do is construct a
sufficiently complex saved query and run multiple instances simultaneously.
Comment 3 Matthew Tuck [:CodeMachine] 2000-10-01 09:13:50 PDT
The only point I have to add is that people who perhaps might not have not been
"privileged" have in the past used this facility to report bugs (numbers escape
me, Dave I think it was you?).  I suspect sanitycheck.cgi has been largely
ignored by mozilla.org.  An implementation of bug #45207 should make this a
non-issue.

We should try to remove as many DoS attacks as possible.

It's possible future versions of Bugzilla could have better indexing to reduce
the load those queries generate, and we could restrict the number of concurrent
queries per IP (3 should be plenty).  Not perfect, but many attackers aren't
very advanced.  I believe the major thing we've seen so far is the occasional
bug stomping, so we could probably get to 99%.
Comment 4 Matthew Tuck [:CodeMachine] 2001-02-20 19:51:02 PST
Interestingly enough, sanity check only appears on the footer if you're in the
"tweakparams" group.
Comment 5 Jesse Ruderman 2001-02-25 13:33:18 PST
See also bug 69616 for creating a new group for the ability to run 
sanitycheck.cgi.
Comment 6 Matthew Tuck [:CodeMachine] 2001-04-10 08:14:46 PDT
Possibly a DOS in quantity -> we'll see about this for 2.14.

I'd be inclined against tightening this up too much on b.m.o though, because
some of us run this on b.m.o and file bugs on it.
Comment 7 Dave Miller [:justdave] (justdave@bugzilla.org) 2001-04-10 08:31:11 PDT
perhaps make you log in for it, and require editbugs privs.  I'm sure most of us 
that actually use it legitimately have editbugs.
Comment 8 Zach Lipton [:zach] 2001-05-28 20:36:56 PDT
editbugs is easy to get, what about another group? cansanity?
Comment 9 Jacob Steenhagen 2001-05-29 13:06:30 PDT
The problem w/creating a system group (esp. just for sanitycheck) is that it
takes away from the number of available product groups.  I think editbugs should
be restrictive enough as it will keep "just anybody" from running it.
Comment 10 Myk Melez [:myk] [@mykmelez] 2001-05-31 15:38:40 PDT
Created attachment 36726 [details] [diff] [review]
restricts sanitycheck.pl to users with "editbugs" privileges
Comment 11 Jacob Steenhagen 2001-06-01 09:10:21 PDT
I find the &&/|| much harder to understand then a simply if/unless block (esp.
in this instance).  Is there a techincal reason for using this syntax?
Comment 12 Myk Melez [:myk] [@mykmelez] 2001-06-01 17:51:24 PDT
There isn't a technical reason for the syntax I used, I just find it cleaner and
more readable in many situations, although I agree that in this case it might be
more logical to break out &confirm_login since that function always returns
either a true value or stops execution.
Comment 13 Myk Melez [:myk] [@mykmelez] 2001-06-01 17:53:48 PDT
Created attachment 36875 [details] [diff] [review]
breaks out confirm_login
Comment 14 Jacob Steenhagen 2001-06-02 07:30:53 PDT
That's easier to read :)
r=jake
Checked In.
Comment 15 Matthew Tuck [:CodeMachine] 2001-07-21 11:38:33 PDT
Bug #69616 wasn't about a new group for sanity checks, but I've filed bug #91761
on it.
Comment 16 Dave Miller [:justdave] (justdave@bugzilla.org) 2001-09-02 23:39:07 PDT
Moving to Bugzilla product

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