Closed
Bug 108812
Opened 23 years ago
Closed 23 years ago
[security] We allow the user to send arbitary SQL!!!!!
Categories
(Bugzilla :: Query/Bug List, defect, P1)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.16
People
(Reporter: bbaetz, Assigned: jacob)
References
()
Details
(Whiteboard: applied to 2.14.1)
Attachments
(1 file)
1.08 KB,
patch
|
bbaetz
:
review+
myk
:
review+
|
Details | Diff | Splinter Review |
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 -
bugzilla.mozilla.org/buglist.cgi?sql=1%3E2)%20OR%20(bugs.bug_id=106544 (%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•23 years ago
|
||
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
installation.
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•23 years ago
|
||
s/warnings/Perl comments/
Updated•23 years ago
|
Priority: -- → P1
Target Milestone: --- → Bugzilla 2.16
Assignee | ||
Comment 3•23 years ago
|
||
Assignee | ||
Comment 4•23 years ago
|
||
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).
Assignee: endico → jake
Assignee | ||
Updated•23 years ago
|
Reporter | ||
Comment 5•23 years ago
|
||
Comment on attachment 56877 [details] [diff] [review]
patch
r=bbaetz for either this patch or one which just deletes it.
Attachment #56877 -
Flags: review+
Comment 6•23 years ago
|
||
Comment on attachment 56877 [details] [diff] [review]
patch
It works. r=myk
Attachment #56877 -
Flags: review+
Comment 7•23 years ago
|
||
Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v <-- buglist.cgi
new revision: 1.153; previous revision: 1.152
done
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 8•23 years ago
|
||
This applied to the 2.14.1 branch with no changes.
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v <-- buglist.cgi
new revision: 1.139.2.1; previous revision: 1.139
Whiteboard: applied to 2.14.1
Comment 9•23 years ago
|
||
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•23 years ago
|
||
Opening security bugs for which fixes have appeared in official bugzilla
release. As per justdave and his posse.
Group: security?
Updated•12 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•