Closed Bug 319091 Opened 14 years ago Closed 14 years ago

disabledtext accepts HTML and thus <script>

Categories

(Bugzilla :: User Accounts, defect)

2.18.4
defect
Not set

Tracking

()

VERIFIED WONTFIX

People

(Reporter: mkanat, Unassigned)

References

Details

From Tim Brown <timb@nth-dimension.org.uk>:

Bugzilla 2.18.4, 2.20

Injection of malicious Javascript is possible by modification of the disabledtext parameter of editusers.cgi.  The contents of this will then be executed on the users browser when they next attempt to login.

---------
I'm honestly not *too* worred about this, since only admins with editusers can changed disabledtext, and having editusers basically makes you a super-admin.

That is, this bug is similar to saying "An admin can install Bugzilla and put a malicious <script> tag on the front page if he wants."

Still, I don't see any reason for disabledtext to allow HTML.
WONTFIX imo. I want the disabledtext to allow live HTML, think
'You've been blocked because of <strong>Ad Spamming</strong> bug 1234.
<a href="/blockpolicy-adspam.html">Please see our policy about this.</a>'.

We allow it for the similar reasoning as for why we allow live HTML in descriptions for classifications, products and components, thus trusting editcomponents people: we trust editusers people not to attempt malicious XSS.
I agree with Marc. Suggesting WONTFIX too. What we really should do is to have a whitelist of allowed HTML tags, as suggested by justdave. But even in this case, I could add a link to a hacked page containing JS in it on the same server, see bug 38862 for a testcase.


<justdave> what we should really do for those fields is put a whitelist of allowed HTML tags and refuse the update if they put ones in that aren't on that list.
<justdave> as for why it's a security issue, that's only because we don't have any form submission security
<justdave> (to ensure that the submit actually came from our form)
Severity: major → normal
OK. I think that perhaps we should keep this around as a "blocker will fix" sort of thing on whitelisting HTML tags.
Target Milestone: Bugzilla 2.16 → ---
disabledtext can only be changed by people who are trusted. I can't see ever "doing an HTML parser and whitelist of tags for disabledtext" being at the top of anyone's priorities. I also suggest WONTFIX.

Gerv
OK. Consensus is very apparently WONTFIX. :-)
Group: webtools-security
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → WONTFIX
While you can use modules like HTML::StripScripts to implement a whitelist, it is still highly non-trivial to get right. Even if you disallow <script>, you still have to be careful to disallow onLoad, onClick, and all the other event handlers which can be added as html attributes. 
Status: RESOLVED → VERIFIED
Adding reporter for bug access.
(In reply to comment #6)
> While you can use modules like HTML::StripScripts to implement a whitelist, it
> is still highly non-trivial to get right. Even if you disallow <script>, you
> still have to be careful to disallow onLoad, onClick, and all the other event
> handlers which can be added as html attributes. 
> 

It's hardly a whitelist if you start talking about disallowing <script>.  The concept of a whitelist is that you explicitly state everything you *will* allow.  You may disagree, but I don't think it acceptable to allow arbitrary code.  I'll give you an example, someone compromises a Bugzilla database, and disables every user with code such as <script>...code to perform local exploit of IE/Mozilla...</script>.  Users try to log in, and they themselves get compromised.  At the very least, Bugzilla is a method for the delivery of a payload, albeit that in this case to inject the payload admin privileges is required.  Of course admin privileges might be quite easy to obtain given the session handling weaknesses in Bugzilla or the existence of https://bugzilla.mozilla.org/show_bug.cgi?id=319085.
Duplicate of this bug: 785699
You need to log in before you can comment on or make changes to this bug.