{allowedTags:null} allows <script>

[mikesamuel]

'<script>alert(1)</script>' ==
sanitizeHtml(
    '<script>alert(1)</script>',
    { allowedTags: null });

'<script>alert(1)</script>' ==
sanitizeHtml(
    '<script>alert(1)</script>',
    { allowedTags: undefined });

The docs say

"What if I want to allow all tags or all attributes?"

Simple! instead of leaving allowedTags or allowedAttributes out of the options, set either one or both to false:

allowedTags: false,
allowedAttributes: false

The internal check checks whether allowedTags is falsey, not false.

Treating null equivalently to false is problematic since null is
much more likely as an output from a function that otherwise
returns an array than false, so treating null and undefined
as equivalent to false is a corner-case with very serious security consequences.

For example,

const MY_POLICY = {
  allowedTags: computeAllowedTags()
};

function computeAllowedTags() {
  if (complexCondition) {
    return INLINE_ELEMENTS;
  } else if (anotherComplexCondition) {
    return BLOCK_AND_INLINE_ELEMENTS;
  } else if (adNauseam) {
    return FORMATTING_ELEMENTS_AND_IMAGES;
  }
  // NOTE: Missing return at bottom implies return of undefined
}

Since the behavior for undefined and null, 0, NaN, "" and other falsey values is not documented, I recommend either

• changing the code that fils in blanks:

  options = extend(sanitizeHtml.defaults, options);

  to first remove any properties with falsey, but non-false values.
• and/or change the falsey checks

  if (options.allowedTags && options.allowedTags.indexOf(name) === -1) {

  to check for false:

  if (options.allowedTags !== false
      && (options.allowedTags || []).indexOf(name) === -1) {

  and similarly for allowedAttributes.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[kedarchandrayan]

Hello @boutell,

Should I pick this up? This seems to be an issue as null or undefined are behaving the same as false. As per the documentation, only false value should be respected.

Also, @mikesamuel has given a very good example in which such mistakes can occur, if a function like computeAllowedTags is being used which misses a default return.

Please let me know your opinion. I will raise a PR if you want.

Thanks,
Kedar Chandrayan

[boutell]

Feel free. Sounds like there's a good reason for a runtime check on this.

[kedarchandrayan]

Hello @boutell,

I have raised the PR as per the discussion. Also added new test cases for covering the expected behavior. Please let me know your comments.

Thanks,
Kedar Chandrayan