Opened 13 years ago

Last modified 9 years ago

#352 confirmed New Feature

Enforce output sanitizing

Reported by: Christer Byström Owned by:
Priority: Normal Milestone:
Component: General Version:
Keywords: Cc:

Description

When an image is drag-and-dropped into the edit field the onmouseover and other event attributes remain intact. There should of course be server-side validation, but currently the XHTML snippets produced are unsafe in themselves and make javascript code injection way too easy.

Change History (8)

comment:1 Changed 13 years ago by Frederico Caldeira Knabben

Keywords: Discussion added

What if one really wants the event attributes to remain untouched? We can't simply strip them.

Also, as as you have said, the injection could be in any case easily done without a proper server side check.

Any proposal?

comment:2 Changed 13 years ago by Christer Byström

That is true. It is a tricky question. Is client-side validation better than no validation at all?

I suppose what's really needed is some kind of validation method/function in the integration modules. For PHP there are XHTML validator modules like KSES. I haven tried it out though.

On the third hand, people might use FCKeditor for trusted users. But I think many people have a false feeling of that the output is safe, and that client-side-validation is sufficient. Is a visible note in the installation instructions enough?

comment:3 Changed 13 years ago by Alfonso Martínez de Lizarrondo

I don't see a real benefit to add the needed code to provide such protection in the client side as it can easily bypassed, and on the other hand it can bring new errors and problems for the people that don't want their event handlers touched.

Besides event handlers you should also look for javascript expressions in the style properties, and so many other things that you really need a server side code to do proper clean up of anything that you allow, and I wouldn't allow anything resembling html from an untrusted source.

comment:4 Changed 12 years ago by Martin Kou

How about making it an option in the server side integration code? Some filtering is definitely needed if you want to put FCKeditor in a public site, like a public forum.

From a brief inspection of source code, KSES seems to work by allowing only a user configured set of XHTML tags and attributes, and disallowing the rest. There are some checks it can do with the attribute values as well. But right now it cannot check for something like this:

<a href="javascript:make_computer_explode();" />

or

<div style="background-image: url(javascript:eval(document.all.my.exploit.code))" />

But I guess these can be prevented by disallowing "javascript:" in risky places.

comment:5 Changed 12 years ago by Frederico Caldeira Knabben

Keywords: Discussion removed
Milestone: FCKeditor 3.0
Summary: lack of img attribute validation makes code injection easyEnforce output sanitizing
Type: BugNew Feature

One important feature that we'll be working on for version 3.0 is "code sanitizing". It will not be a client side feature. It will be part of the server side integration files. It should be possibly extensible by end developers, so they can also define their custom rules.

There are many articles in the web about it. I invite all you to come with nice links we should check when working on it. Thanks.

comment:6 Changed 12 years ago by Wojciech Olchawa

Keywords: Confirmed added

comment:7 Changed 11 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.0CKEditor 3.x

We'll be working on sanitization on future releases, when introducing the server side integrations, which will not be available in the first release.

comment:8 Changed 9 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.x

Milestone CKEditor 3.x deleted

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy