Opened 13 years ago

Closed 11 years ago

Last modified 11 years ago

#2140 closed defect (fixed)

Editor widget vulnerable to XSS attacks

Reported by: guest Owned by: Adam Peller
Priority: high Milestone: 1.2
Component: Editor Version: 0.4.1
Keywords: Cc:
Blocked By: Blocking:

Description

It seems that the editor is bent on executing javascript that a user has entered into an editor and saved. Even if I replace the <script> tags with &lt;script&gt; in the datbase, it executes it when it is displayed in the browser. Dojo should provide a method to disallow user entered javascript from executing.

Attachments (1)

go-btn.gif (564 bytes) - added by Slavon8 4 years ago.
here

Download all attachments as: .zip

Change History (36)

comment:1 Changed 13 years ago by ghendricks@…

Sorry I was not more specific. I am using the Editor2 widget on a <textarea> in a form. The users can enter data, including html and javascript, in the widget and save it then come back to the page and edit it later. Often times users switch to the html mode in the widget (clicking the <h2> button) to paste their html in from another document.

When they save their data, the html chars (<,>,& etc.) get replaced with &lt; &gt; and the like before being loaded back into the form. However when they go to edit the page and their saved data from the database is loaded into the Editor2 widget, any javascript that was pasted in is executed, despite the fact that the html was escaped.

I don't see any way to tell dojo to ignore the <script> tags or any tag for that matter. What I would like to see is a way for the Editor2 to refuse to execute javascript that is loaded into it, or at least honor the fact that I have escaped the data.

comment:2 Changed 13 years ago by liucougar

Resolution: invalid
Status: newclosed

in your server side, you can remove all <script> tag all together easily if you don't want them

comment:3 Changed 13 years ago by liucougar

Milestone: 0.5

comment:4 Changed 13 years ago by guest

Resolution: invalid
Status: closedreopened

I do want them.

The user enters html, including javascript examples. The point is that the Editor is executing them instead of just displaying them. We don't want to remove users data since some may be intentional, not just attack.

Turns out, it doesn't matter if it is entered in the html view. If I enter <script language="javascript">alert("hello");</script> in an Editor it will be executed instead of displayed.

comment:5 Changed 13 years ago by liucougar

Resolution: invalid
Status: reopenedclosed

double escape the content and that tag can be displayed correctly

comment:6 Changed 13 years ago by liucougar

and I think it is your server side script which messed up the content

comment:7 Changed 13 years ago by alex

liucougar, I'm not entirely sold that this isn't our bug. Yes, the server-side could double-escape things, but that doesn't solve the apropos issue, which I suspect boils down to a browser difference between the "escape" values of innerHTML on IE vs. FF. At a minimum, we need to investigate and provide a detailed response and/or workaround.

If the bug is what I think it is, we may not be able to do much, but that doesn't mean we can't provide, at a minimum, some guidance about pre-filtering scripts that could be used.

Regards

comment:8 Changed 13 years ago by alex

Resolution: invalid
Status: closedreopened

comment:9 Changed 13 years ago by liucougar

Owner: changed from anonymous to liucougar
Status: reopenednew

comment:10 Changed 13 years ago by liucougar

Resolution: wontfix
Status: newclosed

I tried in FF/IE:

if you do not escape it, the javascript will be executed

however, adding escape format to the textarea, that won't get executed (when retrieving them back, the escape is gone)

what I suggest you do is: (given you use textarea) populating the textarea with escaped html (means no < or > at all, not only those script part, but all the content)

comment:11 Changed 13 years ago by guest

Resolution: wontfix
Status: closedreopened

I have a similar problem. I want to use the editor2 to edit XML files.

If I initially set the editor content to:

&lt;?xml version='1.0' encoding='ISO-8859-15'?&gt;

... this (at first) will be displayed correctly as:

<?xml version='1.0' encoding='ISO-8859-15'?>

If the user changes to html source mode the editor seem to NOT escape the chars < and > back to &lt; and &gt; as you would expect (since this was the original code) and displays instead:

<?xml version='1.0' encoding='ISO-8859-15'?>

This is problematic for the further processing of the code on the serverside.

comment:12 Changed 13 years ago by guest

An even worse case is the following: THe original source code is:

&lt;Person&gt;Peter&lt;/Person&gt;

displayed as:

<Person>Peter</Person>

back to source-mode:

<Person>Peter</Person>

back to WYSIWYG-mode:

Peter

back to source-mode:

<person _moz-userdefined="">Peter</person>

(Tested in FF2) Maybe there should be an option to tell editor2 to escape < and > characters.

comment:13 Changed 13 years ago by guest

BTW: This problem did not occur in the dojo 4.1 trunk version a couple of weeks ago!

comment:14 Changed 13 years ago by guest

I've also experienced this problem before, but not with the Dojo Editor. The problem is because if a server returns a page with the content wrapped in a textarea, the textarea will interpret normal brackets "<>" as brackets "<>" and escaped brackets "&lt;&gt;" as also brackets "<>". And if you use a textarea to initialize the Dojo Editor, the editor can't tell the difference between escaped text and non-escaped text.

Try it out, put this in a file and open it:

<html>
<textarea>
<h1>What are Headings?</h1>
The heading tag is &lt;h1&gt; ...
</textarea>
</html>

alert(document.getElementsByTagName("textarea")[0].value);

comment:15 Changed 12 years ago by alex

liucougar: why is this still open? If it's a real issue, we need to solve it NOW.

comment:16 Changed 12 years ago by liucougar

the problem described in comment 12 is already fixed in r8069

About the original issue, as described in comment 14, there is no way to distinguish normal <> and escaped ones, I think the editor can only try to prevent script from executing by removing any <script> tags. I will implement a pre/postcontent filter to get rid of any script tags when the content is set, and the script tags will be added back when the editor content is retrieved.

If the the <script> tags is desired to be shown in the editor for the user to edit, the server can escape <script> tags twice so that it will shown in the editor as normal text. But this will likely to conflict with the enter key handling, so I don't really think the user should see the script in the editor window, instead a plugin should be written which places some sorts of placeholder (such as an image) to the editor window for a script, and then popup a window (which contains a textarea) for the user to type in the script content

comment:17 Changed 12 years ago by bill

Milestone: 0.91.1

comment:18 Changed 12 years ago by dylan

Milestone: 1.11.2

mass move of editor issues to 1.2.

comment:19 Changed 12 years ago by Douglas Hays

From a customer:
We escaped the content submitted from editor and save it on server. When view the content (not in editor), it is fine - the escaped script can not be executed. When the content is reloaded to editor, it was unescaped and the script was executed in IE. The expected behavior is to escape the script tags and do not unescape the script tags which is already escaped.

comment:20 in reply to:  19 Changed 12 years ago by Adam Peller

Replying to doughays:

From a customer:

...

As I've stated to said customer, a test case would be helpful.

comment:21 Changed 12 years ago by Adam Peller

So I modified test_Editor.html to include escaped HTML tags. It seems that when the Editor is declared on a DIV, it renders the source as you'd expect. But, with a TEXTAREA, it is un-escaping the text and rendering the HTML.

comment:22 Changed 12 years ago by Adam Peller

(In [13655]) add HTML source examples to tests. Currently fails on TEXTAREA + dijit.Editor. Refs #2140

comment:23 Changed 12 years ago by bill

Component: GeneralEditor
Priority: highestnormal

comment:24 Changed 12 years ago by Adam Peller

Priority: normalhigh
severity: normalmajor

comment:25 Changed 12 years ago by liucougar

as there is no way to tell <> from their escaped chars in textarea value, the only way to work around it I can think of is to escape the content of textarea by server (This can not be done in client side)

so say you have this in a page:

<textarea><p>should see html here &lt;INPUT TYPE="IMAGE" SRC="javascript:alert('no scripting attacks')"&gt;<p></textarea>

it should be escaped to look like this:

<textarea>&lt;p&gt;should see html here &amp;lt;INPUT TYPE="IMAGE" SRC="javascript:alert('no scripting attacks')"&amp;gt;&lt;p&gt;</textarea>

so if you decided to use textarea, you have to escape the content before you write it out to the html page

comment:26 Changed 12 years ago by Adam Peller

Owner: changed from liucougar to Adam Peller
Status: reopenednew

yeah, I came to the same conclusion. So I talked it over with wildbill and we decided to just deprecate the use of textarea + dijit.Editor with a console warning. Use div instead, and there's no problem. There seems to be no real need to use textarea, except for the unobtrusive HTML thing, but it seems so troublesome here, perhaps it's best avoided. If there is strong demand, we could throw in a switch to disable the console warning -- a "I know what I'm doing" switch.

comment:27 Changed 12 years ago by liucougar

textarea does have one advantage: if you use div, you won't be able to retrieve href/src attributes as they are set.

for example:

<div dojoType="dijit.Editor">
<a href="/test">test</a>
</div>

in IE, the editor will actually have the following content:

<div dojoType="dijit.Editor">
<a href="http://your.server.com/test">test</a>
</div>

(the href is converted to a full path including host name).

There are other cases where the browser will happily change the href/src attributes with no way to find out what's the original values are

comment:28 Changed 12 years ago by Adam Peller

(In [13699]) Deprecate RichText? + TEXTAREA. Refs #2140 !strict Provide console warning and update docs and tests. Note: textarea is still mentioned in the code as a savearea.

comment:29 Changed 12 years ago by Adam Peller

yeah, the href thing requires a separate solution. Didn't we have a djRealUrl for this purpose?

comment:30 Changed 12 years ago by liucougar

if you use div, you can't detect what's the original value is, so djRealUrl can not be set to the correct original value

comment:31 Changed 11 years ago by Adam Peller

see #344

comment:32 in reply to:  27 Changed 11 years ago by bill

Replying to liucougar:

textarea does have one advantage: if you use div, you won't be able to retrieve href/src attributes as they are set.

IIUC, this is only an issue with relative URLs, like "foo.html" or "/boo/baz.html", not full URLs like "http://www.dojotoolkit.org/". I'm only interested in supporting full URLs.

comment:33 Changed 11 years ago by Adam Peller

Resolution: fixed
Status: newclosed

Closing, as decreed by Bill. It sounds like there may be some need to continue using textarea under certain circumstances, and I suppose users could continue to do so at their own risk (and someone could file a request for a flag to turn off the console warning) Does anyone have any other suggestions?

comment:34 Changed 11 years ago by bill

(In [15346]) Use <div> not <textarea> for Editor, since <textarea> is deprecated. Refs #2140

comment:35 Changed 11 years ago by bill

(In [15347]) oops, in [15346] accidentally removed editor's id parameter. refs #2140.

Changed 4 years ago by Slavon8

Attachment: go-btn.gif added
Note: See TracTickets for help on using tickets.