Guide for how to sanitize and serialize data with hooks about docs HOT 9 CLOSED

Yep, new generator is using helmet now. I also think we have enough information and examples for how to use hooks for sanitization (e.g. processing chat message data), the validation common hooks and the new hooks documentation.

from docs.

@ekryski I've implemented sanitization in my app. While there's lots of ways to do it, I'm using DOMPurify in a before all hook (the sanitize() function below) that I apply to each one of my services.

The hook itself is pretty straightforward:

const jsdom = require('jsdom');
const windowShim = jsdom.jsdom('', {
  features: {
    FetchExternalResources: false,
    ProcessExternalResources: false
  }
}).defaultView;
const DOMPurify = createDOMPurify(windowShim);

/* Sanitizes all string input fields to prevent XSS attacks.
 * Removes all HTML elements and <script> tags.
 */
export function sanitize() {
  return function (hook) {
    const data = _.get(hook, 'data');
    if (_.isObject(data)) {
      Object.keys(data).forEach(function (key) {
        let value = data[key];
        if (_.isString(value)) {
          data[key] = DOMPurify.sanitize(data[key], {ALLOWED_TAGS: []});
        }
      });
    }
    return hook;
  };
}

I'd be happy to add a page to the documentation on the topic of sanitization if you'd like. I haven't done any serialization so I couldn't write about that. Let me know and I can begin work on a PR.

from docs.

@petermikitsh this is great! I think this is something we might just modify slightly and roll into feathers-hooks-common. Thanks for sharing!

from docs.

Yeah, there are certainly some modifications you'd want to make for greater reuse value:

• Deep objection sanitization. My implementation doesn't inspect deeply nested objects because I'm using sql, which doesn't for allow nested objects. I'm sure feathers users with NoSQL backends would want that.
• Control the level of sanitization. In my use case, I wanted all HTML tags stripped, so I set ALLOWED_TAGS: []. Some people might want something less invasive, like removal of only <script> tags / XSS-specific vectors. So you could pass that in as an option to the hook.

from docs.

This is specifically for sanitizing HTML using the DOM right? I'm not sure if we can roll it into feathers-hooks-common without adding JSDom as a dependency (unless we pass the window reference as options).

from docs.

There are headless XSS sanitizers, such as https://github.com/leizongmin/js-xss that don't require a DOM.

from docs.

Something else you might want to consider is simply having the xss sanitization hooks as mandatory, or design them such that you must explicitly opt-out if you don't want them.

But i'm conflicted. Feathers is supposed to be this non-opinionated, lightweight layer. When it comes to the feathers-authentication module, we obviously take security concerns seriously. Is it too 'framework-y' to be automatically removing <script> tags from data persisted via Feathers services? I don't think there is a clear answer to that. The answer lies in more of what people expect out of Feathers.

If my recollection is correct, Meteor.js does not do any sanitization, whereas Rails does. I'm unsure of other web application frameworks beyond that. Since Feathers is so generic, you be using it to build an exclusively native application, in which case, XSS isn't even an attack vector as there is no DOM.

Conversely, there is a case to be made by simply saying Feathers is 'secure by default'. We don't know if you're using feathers to build web apps or native apps (or hybrid), but we'll protect you no matter your use case. And since anyone who is using feathers for web apps should have XSS protection (I can't imagine anyone not wanting it), we might as well implement it once, implement it right, and not expect everyone using feathers to spend time crafting their own XSS protection and having to enable it via hooks on all of their services.

@daffl @ekryski thoughts?

from docs.

I'd like to see something along the lines of helmut for Feathers where it's just a few lines to implement better security. With our new app-level hooks it would be quite easy to opt in to sanitization.

from docs.

Well I mean for right now you can just use helmut, which is great! I hadn't heard of it. It should be used by default when generating an app I think.

from docs.