Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Style attribute on anchor tag throws an exception instead of error #36

Closed
TDA opened this issue Jul 29, 2015 · 5 comments
Closed

Style attribute on anchor tag throws an exception instead of error #36

TDA opened this issue Jul 29, 2015 · 5 comments
Labels

Comments

@TDA
Copy link

TDA commented Jul 29, 2015

Steps to reproduce:

var options = {
        whiteList: {
          a: ['href', 'id', 'style'],
          em: [],
          span: ['id', 'tabindex'],
          strong: []
        }

and override the onTagAttr, like so:

        onTagAttr: function (tag, name, value, isWhiteAttr) {
            if (isWhiteAttr && xss.safeAttrValue(tag, name, value) === '') {
              grunt.log.error('%s: INVALID VALUE FOR ATTRIBUTE <%s %s="%s">', src, tag, name, value);
              hasErrors += 1;
              errorFound = true;
              return '';
            }
          }

run this over a html file with contents:
<a href='' style='color: #0095dd; text-decoration: none;'>Whatever text</a>

Expected Behaviour:

  1. The xss module logs an error

Actual Behaviour:
Warning: Cannot call method 'process' of undefined Use --force to continue.

@pdehaan

@pdehaan
Copy link

pdehaan commented Jul 29, 2015

Steps to reproduce:

var xss = require('xss')

var html = "<a href='https://foo.bar/' style='color: #0095dd; text-decoration: none;'>Whatever text</a>"

var options = {
  whiteList: {
    a: ['href', 'style']
  },
  onTagAttr: function (tag, name, value, isWhiteAttr) {
    if (isWhiteAttr && xss.safeAttrValue(tag, name, value) === '') {
      console.log('INVALID VALUE FOR ATTRIBUTE <%s %s="%s">', tag, name, value)
      return ''
    }
  }
}

var myxss = new xss.FilterXSS(options)
console.log(myxss.process(html))

Output:

➜  fxa-travis  node xss-test.js

/Users/pdehaan/dev/fxa-travis/node_modules/xss/lib/default.js:183
    value = cssFilter.process(value);
                      ^
TypeError: Cannot call method 'process' of undefined
    at Function.safeAttrValue (/Users/pdehaan/dev/fxa-travis/node_modules/xss/lib/default.js:183:23)
    at options.onTagAttr (/Users/pdehaan/dev/fxa-travis/xss-test.js:10:28)
    at /Users/pdehaan/dev/fxa-travis/node_modules/xss/lib/xss.js:149:19
    at addAttr (/Users/pdehaan/dev/fxa-travis/node_modules/xss/lib/parser.js:130:19)
    at parseAttr (/Users/pdehaan/dev/fxa-travis/node_modules/xss/lib/parser.js:148:11)
    at /Users/pdehaan/dev/fxa-travis/node_modules/xss/lib/xss.js:145:23
    at parseTag (/Users/pdehaan/dev/fxa-travis/node_modules/xss/lib/parser.js:79:22)
    at FilterXSS.process (/Users/pdehaan/dev/fxa-travis/node_modules/xss/lib/xss.js:123:17)
    at Object.<anonymous> (/Users/pdehaan/dev/fxa-travis/xss-test.js:18:19)
    at Module._compile (module.js:456:26)

Looks like the error is coming from /lib/default.js:183.

@pdehaan
Copy link

pdehaan commented Jul 29, 2015

Potential workaround is to explicitly specify a cssFilter in our xss.safeAttrValue() call:

xss.safeAttrValue(tag, name, value, myxss.cssFilter)

Semi-related, we can do CSS attribute filtering by passing a css white list in the options object:

var options = {
  whiteList: {
    a: ['href', 'style']
  },
  onTagAttr: function (tag, name, value, isWhiteAttr) {
    var attrValue = xss.safeAttrValue(tag, name, value, myxss.cssFilter)
    if (isWhiteAttr && value === '') {
      console.log('INVALID VALUE FOR ATTRIBUTE <%s %s="%s">', tag, name, value)
      return ''
    }
  },
  css: {
    whiteList: {
      'color': true,
      'text-decoration': true
    }
  }
}

This module is awesome!

@pdehaan
Copy link

pdehaan commented Jul 30, 2015

So I think the fix for this is to check if cssFilter argument is defined is safeAttrValue() in /lib/default.js:142-144:

var FilterCSS = require('cssfilter').FilterCSS;

...

function safeAttrValue (tag, name, value, cssFilter) {
  cssFilter = cssFilter || new FilterCSS();

  // 转换为友好的属性值,再做判断
  value = friendlyAttrValue(value);

leizongmin added a commit that referenced this issue Jul 30, 2015
@leizongmin
Copy link
Owner

@pdehaan @TDA Fixed this problem, please install the new version xss@0.2.3 Thank you! :-)

@leizongmin leizongmin added the bug label Jul 30, 2015
@TDA
Copy link
Author

TDA commented Jul 30, 2015

@leizongmin Thank you so much!! That was fast!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants