Last Comment Bug 108812 - [security] We allow the user to send arbitary SQL!!!!!
: [security] We allow the user to send arbitary SQL!!!!!
applied to 2.14.1
Product: Bugzilla
Classification: Server Software
Component: Query/Bug List (show other bugs)
: 2.15
: All All
P1 blocker (vote)
: Bugzilla 2.16
Assigned To: Jacob Steenhagen
: default-qa
Depends on:
Blocks: 103885
  Show dependency treegraph
Reported: 2001-11-06 22:49 PST by Bradley Baetz (:bbaetz)
Modified: 2012-12-18 20:46 PST (History)
7 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---

patch (1.08 KB, patch)
2001-11-07 06:41 PST, Jacob Steenhagen
bbaetz: review+
myk: review+
Details | Diff | Splinter Review

Description User image Bradley Baetz (:bbaetz) 2001-11-06 22:49:18 PST
So, buglist.cgi has an sql paramater. And, as long as that doen't contain a
semicolon, we send it. Oops.

The above url will allow you to see the title of a test security group bug. Its
more complicated than it looks - (%3E is
>, and %20 is space) would work as well, except that then we join the users
table cubed then, without conditions, and that takes a long long long long long
time on bmo.

This was presumably added for advanced query support, or something. Lets kill
it. Its a huge hole, and even if we someone patch it up it sitll makes me nervous.

This turned up another bug, which I discovered over the weekend - my
SelectVisible stuff does "WHERE (security conditions) AND other stuff" rather
than "WHERE (security conditions) AND (other stuff)". That wouldn't have helped
in this case, though, because the extra bracketing could have been easily
removed. Fixing that may be interesting, since the where clause isn't the end -
we have to search for HAVING/GROUP BY in the rest of the where, while still
letting people search for bugs containing the word having. I have no idea how to
do that. I was vaguely aware of this when I wrote the support, but all places
which use that code use AND. If someone can tell me how to fix it, I will.
Comment 1 User image Matthew Tuck [:CodeMachine] 2001-11-06 22:57:36 PST
OK, options here appear to be one or more of:

1. Commenting it out and giving appropriate end-of-the-world warnings. I'm
hesitant to remove it as someone might have a use for it and have a closed
2. Properly parsing the SQL condition to ensure it is valdi. A bracket matching
algorithm would work out if the bracket count went below 1, however GROUP BY
does sound like it's another exploit.
3. Restricting usage of this parameter to some appropriate trusted group.
Comment 2 User image Matthew Tuck [:CodeMachine] 2001-11-06 23:05:27 PST
s/warnings/Perl comments/
Comment 3 User image Jacob Steenhagen 2001-11-07 06:41:03 PST
Created attachment 56877 [details] [diff] [review]
Comment 4 User image Jacob Steenhagen 2001-11-07 06:44:35 PST
My personal preference would be to just yank the code altogether.  However, I
instead followed Matthew's reccomendation and commented it out w/a prepended
warning (perl comment).
Comment 5 User image Bradley Baetz (:bbaetz) 2001-11-07 08:13:17 PST
Comment on attachment 56877 [details] [diff] [review]

r=bbaetz for either this patch or one which just deletes it.
Comment 6 User image Myk Melez [:myk] [@mykmelez] 2001-11-07 14:54:01 PST
Comment on attachment 56877 [details] [diff] [review]

It works. r=myk
Comment 7 User image Myk Melez [:myk] [@mykmelez] 2001-11-07 16:53:48 PST
Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.153; previous revision: 1.152
Comment 8 User image Dave Miller [:justdave] ( 2001-11-17 00:12:24 PST
This applied to the 2.14.1 branch with no changes.

/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision:; previous revision: 1.139
Comment 9 User image Dave Miller [:justdave] ( 2001-12-10 17:26:56 PST
Hmm, it seems the bulk change thinks I'm not changing anything if all I do is
add names to the CC list, so I guess I have to make a comment.  Anyhow, adding
the representatives from the organizations we know of that support Bugzilla
distributions so they're aware of our upcoming security release
Comment 10 User image Mike Shaver (:shaver -- probably not reading bugmail closely) 2002-01-05 16:02:06 PST
Opening security bugs for which fixes have appeared in official bugzilla
release.  As per justdave and his posse.

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