[Kss-devel] Escaping of replaceInnerHTML() output

[Martin Aspeli]

Hi guys,

I'm working on inline editing for content that uses formlib-based edit 
forms. It's actually working now in 
http://dev.plone.org/plone/browser/plone.app.form/branches/fschulze-optilude-usw

The approach is a bit different to the Archetypes version, though. One 
difference is that instead of calling a macro to render the output I get 
the raw output value as a string (from an IDisplayWidget) and then use 
replaceInnerHTML to place it into the correct node in the page.

However, I need to escape HTML properly. If you enter some HTML during 
inline editing, it shouldn't be rendered verbatim (as if it were 
inserted with structure:). This could potentially cause XSS type 
attacks, for example (even if it will be fixed once the page is reloaded 
and the normal view logic is used in this case). Only in some 
circumstances, such as for Text fields, should it come out that way.

Now, the IDisplayWidget actually takes care of this. I get a string such 
as '&lt;b&gt;Bold title&lt;/b&gt;' if I enter '<b>Bold title</b>' into 
the inline edit box. I then pass that to replaceInnerHTML() in the core 
command set.

Unfortunately, by the time this hits the DOM, it's been un-escaped. I 
can't exactly see how or where, but it seems that somethings translates 
those entities into proper characters again.

Digging around, I see that 
kss.core.plugins.core.commands.KssCoreCommands has this code:

def replaceInnerHTML(self, selector, new_value, withKssSetup='True'):
         new_value = HtmlParser(new_value)().encode('ascii', 
'xmlcharrefreplace')
         command = self.commands.addCommand('replaceInnerHTML', selector)
         data = command.addParam('html', new_value)
         data = command.addParam('withKssSetup', withKssSetup)

So, let's say the title I entered was "<b>Bold title</b>" as the input. 
The method is called with new_value = '&lt;b&gt;Bold title&lt;/b&gt;'. 
The first line of that function turns it into '&#60;b&#62;Bold 
title&#60;/b&#62;'.

In kss.core.parsesr.HtmlParser, we have:

     def __call__(self):
         value = unicode(self.soup)
         # Replace named HTML entitied in each case.
         # This is necessary for two reasons:
         # 1. Fixes an IE bug.
         # 2. Needed for the alternate transport mechanism to work.
         value = replace_html_named_entities(value)

         return value

Funnily enough, the last line turns this into u'&#60;b&#62;Bold 
title&#60;/b&#62;'. I don't know what the signifiance of the the two 
comments are.

This is the relevant part of the KSS reponse sent to the client:

<kukit:command selector="title-display"
                name="replaceInnerHTML" selectorType="htmlid">

     <kukit:param name="html">&#60;b&#62;Bold 
title&#60;/b&#62;</kukit:param>

     <kukit:param name="withKssSetup">True</kukit:param>

</kukit:command>

At this point, I'm not quite sure what happens. The plugin code looks 
like this:

kukit.actionsGlobalRegistry.register('replaceInnerHTML', function(oper) {
/*
*  accepts both string and dom.
*/
;;; oper.componentName = '[replaceInnerHTML] action';
     oper.evaluateParameters(['html'], {'withKssSetup': true});
     oper.evalBool('withKssSetup');
     var node = oper.node;
     var insertedNodes;
     if (typeof(oper.parms.html) == 'string') {
         node.innerHTML = oper.parms.html;
         insertedNodes = [];
         for (var i=0; i<node.childNodes.length; i++) {
             insertedNodes.push(node.childNodes[i]);
         }
     } else {
         oper.parms.html = kukit.dom.forceToDom(oper.parms.html);
         kukit.dom.clearChildNodes(node);
         insertedNodes = kukit.dom.appendChildren(
             oper.parms.html.childNodes, node);
     }
;;; kukit.logDebug(insertedNodes.length + ' nodes inserted.');
     if (oper.parms.withKssSetup) {
         kukit.engine.setupEvents(insertedNodes);
     }
});
kukit.commandsGlobalRegistry.registerFromAction('replaceInnerHTML',
     kukit.cr.makeSelectorCommand);

I assume there's something dodgy going on when you call node.innerHTML.

Is this a bug in KSS?

Martin

-- 
Author of `Professional Plone Development`, a book for developers who
want to work with Plone. See http://martinaspeli.net/plone-book