knownasilya / ember-toggle

Checkbox based Toggle Switches for Ember
http://knownasilya.github.io/ember-toggle/
MIT License
112 stars 52 forks source link

Warning: Binding style attributes may introduce cross-site scripting vulnerabilities #95

Closed FredUK closed 7 years ago

FredUK commented 7 years ago

I'm getting this warning everywhere I have a toggle on my site:

WARNING: Binding style attributes may introduce cross-site scripting vulnerabilities; please ensure that values being bound are properly escaped. For more information, including how to disable this warning, see http://emberjs.com/deprecations/v1.x/#toc_binding-style-attributes.

it tries to set the value "touch-action: manipulation; -ms-touch-action: manipulation; cursor: pointer; display: none;" into the style attribute of the on/off label element.

However I can't seem to track where this is exactly happening in ember-toggle.

lolmaus commented 7 years ago

Hmm, I've introduced drag-to-toggle support in 5.1 with ember-gestures. But I didn't modify the label element, only the switch: https://github.com/knownasilya/ember-toggle/compare/v5.0.2...v5.1.1 .

The problem doesn't happen in the demo app, so it might be caused by a combination of Ember addons or by a browser extension.

Can you please try running your app with all browser extensions disabled?

FredUK commented 7 years ago

Hi, thanks for your reply. I tried removing all browser extensions but problem still persisted. I tried debugging it to see which addon was causing it and found out it had something to do with ember-hammertime/mixins/touch-action. Going back on the call stack I found this:

Where it triggers deprecation warning: image

The CSS value trying to be set: image ...

The function trying to set the value: image .... The Environment property for that function: image

As you can see that seems to be referring to the x-toggle-label. There's an attribute binding named 'touchActionStyle:style'.

I've checked my app and the compiled vendor.js file mentions:

define('ember-hammertime/mixins/touch-action', ['exports'], function (exports) {
  'use strict';

  var computed = Ember.computed,
      defineProperty = Ember.defineProperty,
      get = Ember.get,
      Mixin = Ember.Mixin,
      _Ember$String = Ember.String,
      htmlSafe = _Ember$String.htmlSafe,
      isHTMLSafe = _Ember$String.isHTMLSafe;

  var FocusableInputTypes = ['button', 'submit', 'text', 'file'];
  var TouchActionSelectors = ['button', 'input', 'a', 'textarea'];
  var TouchActionProperties = 'touch-action: manipulation; -ms-touch-action: manipulation; cursor: pointer;';

  function touchActionStyle() {
    var style = get(this, 'touchActionProperties');
    var otherStyleKey = get(this, 'otherStyleKey');

    if (otherStyleKey) {
      var otherStyle = get(this, otherStyleKey);

      if (otherStyle) {
        if (isHTMLSafe(otherStyle)) {
          otherStyle = otherStyle.string;
        }
        style += otherStyle;
      }
    }

    return htmlSafe(style);
  }

I notice ember-toggle has ember-hammertime has a dependency. I'm not sure if this functionality is intentional or why it is being triggered just for me but it does not appear to be related to my app. Perhaps some combination of Ember toggle settings?

It appears ember-hammertime is trying to hook up to ember-toggle labels and trying to bind the style-attribute to set some css for touch support ? Do you think you could shed any light?

Thanks in advance.

lolmaus commented 7 years ago

I have introduced ember-hammertime as a dependency to ember-toggle in order to support toggling with a gesture (#49).

In my app, I do see those touch-action styles applied to ember-toggle elements just as you do, but it doesn't produce warnings for me.

It looks like a problem upstream. Note how touchActionStyle returns htmlSafe(). htmlSafe's sole purpose is to prevent the warning. Yet, in your debug output the value is a regular non-htmlSafe string.

Not sure why this happens. Can you please file an issue to ember-hammertime?

knownasilya commented 7 years ago

What versions of the CLI and Ember are you both using?

FredUK commented 7 years ago
"ember-cli": "2.6.2"
"ember": "2.8.3"

I tried ember-toggle with "ember new testapp" and it worked fine. I will now try with the same versions of the app where I am getting the deprecations to see if it triggers it. If it does not, I will need to start installing one addon at a time to see if that's the issue.

knownasilya commented 7 years ago

Maybe see when hammertime fixed its deprecation, and do npm ls ember-hammertime in your project to see if there are any conflicting versions.

lolmaus commented 7 years ago

@FredUK Oh, then try this: https://github.com/html-next/ember-hammertime/issues/29

@knownasilya We're on 2.12, in the process of updating to 2.15.

FredUK commented 7 years ago

Hi @knownasilya , the only thing using ember-hammertime is ember-toggle:

-- ember-toggle@5.1.2
  -- ember-hammertime@1.2.4

@lolmaus Thanks for the suggestion. I have tried that and it didn't fix it seeing I'm running Ember 2.8.3: ember-string-ishtmlsafe-polyfill is not required for Ember 2.8.0 and later, please remove from yourpackage.json.

I have uninstalled about 8 or 9 packages but it's still throwing the deprecation warning. Just for the sake of it I'm just going to drop the npm and bower .json files here in case you guys can easily spot any package that might be causing it. If not don't worry I'll keep looking. Thanks for your help.

"devDependencies": {
    "broccoli-asset-rev": "^2.4.2",
    "ember-ajax": "^2.0.1",
    "ember-breadcrumbs": "0.1.9",
    "ember-can": "0.8.0",
    "ember-cli": "2.6.2",
    "ember-cli-app-version": "1.0.0",
    "ember-cli-autoprefixer": "0.4.1",
    "ember-cli-babel": "^5.1.6",
    "ember-cli-chai": "0.2.0",
    "ember-cli-dependency-checker": "^1.3.0",
    "ember-cli-document-title": "0.3.3",
    "ember-cli-dotenv": "1.0.4",
    "ember-cli-flash": "1.3.16",
    "ember-cli-htmlbars": "^1.0.8",
    "ember-cli-htmlbars-inline-precompile": "^0.3.2",
    "ember-cli-inject-live-reload": "^1.4.0",
    "ember-cli-mocha": "0.13.0",
    "ember-cli-moment-shim": "3.1.0",
    "ember-cli-sass": "^4.0.0-beta.7",
    "ember-cli-string-helpers": "1.4.0",
    "ember-cli-test-loader": "^1.1.0",
    "ember-cli-uglify": "1.2.0",
    "ember-composable-helpers": "2.0.0",
    "ember-data": "2.10.0",
    "ember-data-model-fragments": "2.3.3",
    "ember-export-application-global": "^1.0.5",
    "ember-i18n": "4.4.0",
    "ember-ignore-children-helper": "1.0.0",
    "ember-inline-svg": "0.1.11",
    "ember-load-initializers": "^0.5.1",
    "ember-moment": "4.2.1",
    "ember-resolver": "^2.0.3",
    "ember-toggle": "5.1.2",
    "ember-truth-helpers": "1.2.0",
    "jquery": "2.1.4"
  },
  "dependencies": {
    "loader.js": "^4.0.1"
  }

// bower
{
  "dependencies": {
    "ember": "2.8.3",
    "ember-cli-shims": "~0.1.1",
    "ember-cli-test-loader": "0.2.2",
    "ember-cli-moment-shim": "0.0.3",
    "ember-mocha-adapter": "~0.3.1",
    "eventemitter2": "0.4.14",
    "jquery": "2.1.4",
    "sinonjs": "1.14.1",
    "moment": "2.9.0",
    "moment-timezone": "0.5.13"
  },
  "resolutions": {
    "moment": "2.9.0",
    "jquery": "2.1.4"
  }
}
lolmaus commented 7 years ago

@FredUK I wonder if upgrading Ember CLI will do the trick. I mean, you're still gonna do it some day.

Anyway, I believe this issue discussion has to be moved to the ember-hammertime issue queue.

FredUK commented 7 years ago

Alright, I'll do the ember-cli upgrade. Feel free to close this. Thanks a lot for your help.

FredUK commented 7 years ago

I feel I should add how I fixed it in case someone else comes across the same issue: The issue was gone when moving from Ember 2.9.1 to 2.10.2