XSS protection: userland formatters, etc. can output text which is inherently dangerous. Define what we protect, how and where. Lists risks.

[GerHobbelt]

For one, we cannot (and WILL NOT) filter cell node formatting as we intend the userland code to be able to produce any HTML they desire to be placed in each grid cell.

On the other hand, we can do something about DOM node attributes, e.g. tooltip/title values to ensure those adhere to standards. However, even there, there's the 'risk' of making it harder for userland code producing DOM node attribute content which is targeting advanced tooltip-like systems such as intro.js.

[6pac]

Is there any point worrying about this on the client side anyway? The client has arbitrary control to circumvent anything we do.

[GerHobbelt]

Yes and no. In our app there are situations which are solved cleaner and
leaner when SlickGrid has a clearly defined protocol for this.

In particular: having SlickGrid ensure that Dom attribute values for
cells get encoded at all times allows user land code to be agnostic about
how user data will be displayed by the cell formatters exactly: in cell Dom
values and/or Dom attribute values.

It's a bit complicated as it's not SlickGrid alone.

See this as a loud reminder to myself to ensure that SlickGrid offers a
userland data assumption which allows a large app like ours to have its
dataview be agnostic about the exact destination in the Dom through the
(custom) cell formatters.

SlickGrid is involved itself since I use the SlickGrid internal way of
adding attributes (classes and others) to the SlickGrid cell Dom node. That
means my render path from data to Dom differs slightly depending on exact
Dom destination and currently requires the data feed to be aware about
where it goes and that is in itself Bad Design.

Since the bits involved are right smack in my own patch work on SlickGrid I
think the concern is first in my own clone and that's why I filed the issue
locally.

I'm not near a machine right now otherwise i'ld be able to pinpoint the
issue paths more clearly to you, sorry for that. :-(
On Oct 7, 2015 3:26 AM, "Ben McIntyre" notifications@github.com wrote:

Is there any point worrying about this on the client side anyway? The
client has arbitrary control to circumvent anything we do.

—
Reply to this email directly or view it on GitHub
#18 (comment)
.

[GerHobbelt]

TL;DR the above: this is a documentation thing mostly.

Q: what do you have to do in your userland code and what can you expect from SlickGrid re output-to-UI behaviour anywhere? escaping? If so, then where, how, what is and what isn't?

A:

• for cell/row/header DOM nodes' attributes, SlickGrid escapes single- and double-quotes, '&', '<', '>' and any newline but nothing else for all attributes you set via your formatter function, metadata or SlickGrid options; reviewers should please refer to the internal appendMetadataAttributes function and the 'core' component HtmlEntities's encode() method for exact implementation details.
  Note that attribute names are printed as-is, only the attribute values are escaped.
• formatters may produce arbitrary HTML which will be included in the DOM as-is. Any data formatting/escaping/transformation/protection has to be handled by the assigned formatter.
  Reviewers should please refer to the internal mkCellHtml function, which at the end invoked the (possibly custom) formatter, which is assumed to produce a chunk of HTML into info.html at the line info.html = getFormatter(row, cell)(row, cell, value, m, rowDataItem, info);: this output will be included in the DOM as-is in internal functions appendCellHtml() and updateElementHtml() -- where the latter uses jQuery .html() while the former outputs this as part of a larger DIV construction and writes it to the DOM in cleanupAndRenderCells() by executing x.innerHTML = stringArray.join("");

Note that SlickGrid formatters invoked through mkCellHtml et al may modify/augment the attributes/CSS classes/CSS styles of the containing cell DIV node: all these changes to attributes will either travel through appendMetadataAttributes as described above before landing in the DOM or are processed by the internal updateElementHtml() function which employs jQuery's .attr(key, val) API to set the attributes: then only jQuery internal attribute escaping rules apply.

This needs to be unified so that the data-to-UI flow paths are no more either-or and all produce the same output, guaranteed.

Note to self: base/derive final docs on the above blurb

[GerHobbelt]

Side note: a nice test vector for your app using SlickGrid is to enter or inject:

<script>debugger;</script><script>alert("ka'boom!");</script>

with the dev panel open:

• the basic XSS holes will show up as 'debugger' breakpoints and
• the separate script tags around the quoted kaboom alert ensure that the entire vector will still deliver, even when the vector ends up as an attribute value when the value isn't properly escaped to prevent that single or double quote from inadvertently terminating the attribute value prematurely: watch devpanel errors about the DOM too!

When the test vector triggers, you can bet your userland app code is not doing what it should to protect your app & users.