Jemt / Fit.UI

Fit.UI is a JavaScript based UI framework built on Object Oriented principles
http://fitui.org
GNU Lesser General Public License v3.0
19 stars 7 forks source link

Fit.Template - protect against code injection #68

Closed Jemt closed 5 years ago

Jemt commented 5 years ago

Consider implementing support for code injection protection. Like React, it should be enabled by default, although that would break compatibility.

Implementation suggestions.

Suggestion 1 Add an alternative to the Content property so we have two.

t.Content.Headline = "<h1>Hello</h1>";
t.UnsafeContent.Headline = "<h1>Hello</h1>"; // <h1> will be interpreted

Suggestion 2 Require use of special object for safe/unsafe content:

t.Content.Headline = new Fit.Template.SafeString("<h1>Hello</h1>");
t.Content.Headline = new Fit.Template.UnsafeString("<h1>Hello</h1>");

Obviously we already kind of have that with support for DOM elements:

t.Content.Headline = Fit.Dom.CreateElement("<h1>Hello</h1>");

So if that's sufficient, ordinary strings should just be encoded and safe by default while DOM objects allow for unsafe content.

Suggestion 3

t.AllowUnsafeContent(false);
t.Content.Headline = "<h1>Hello</h1>"; // <h1> won't be interpreted now

AllowUnsafeContent should be True by default in the 1.X branch and emit a message in the console if the property is not explicitly set, warning that the default behaviour will change in the future. The only way to ensure compatibility with future versions is to explicitly set AllowUnsafeContent to either True of False.

Perhaps AllowUnsafeContent should be renamed to AllowUnsafeStrings(boolean) to make it very obvious what gets encoded and what don't.

Jemt commented 5 years ago

Test code

debug.html

<h1>{[Headline]}</h1>

<!-- LIST Controls -->
<div style="margin: 1em;">
    {[Control]}
</div>
<!-- /LIST Controls -->

test.js

var view = new Fit.Template(true);
//view.AllowUnsafeContent(false);
view.LoadUrl("debug.html", function(sender)
{
    view.Content.Headline = "<span style='color:red'>Test hack</span>";
    view.Content.Controls.Clear();
    var i = view.Content.Controls.AddItem();
    i.Control = "<b>Hej</b><img src='https://sitemagic.org/files/images/Bird.png' onload='console.log(`hack`)'>";
    var i2 = view.Content.Controls.AddItem();
    i2.Control = Fit.Dom.CreateElement("DOM <b style='color:red; font-size: 1.75em;'>always</b> allow for code injection no matter the value of AllowUnsafeContent");
    view.Update();
});
view.Render(document.body);

Testing without explicitly setting AllowUnsafeContent should result in a warning in the browser console. Setting AllowUnsafeContent to False should result in HTML being display as is, and setting AllowUnsafeContent to True should result in HTML being interpreted. DOM always allow for code injection of course.


image

image


image

image


image

image