Merge pull request #577 from chalotrekking/main

For allowedTags, stop treating falsy values other than false, same as false.

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased
+
+- If allowedTags is falsy but not exactly `false`, then do not assume that all tags are allowed. Rather, allow no tags in this case, to be on a safer side. This fixes [issue #176](https://github.com/apostrophecms/sanitize-html/issues/176).
+
 ## 2.7.2 (2022-09-15)
 
 - Closing tags must agree with opening tags. This fixes [issue #549](https://github.com/apostrophecms/sanitize-html/issues/549), in which closing tags not associated with any permitted opening tag could be passed through. No known exploit exists, but it's better not to permit this. Thanks to 

index.js

@@ -117,7 +117,7 @@ function sanitizeHtml(html, options, _recursing) {
   // vulnerableTags
   vulnerableTags.forEach(function (tag) {
     if (
-      options.allowedTags && options.allowedTags.indexOf(tag) > -1 &&
+      options.allowedTags !== false && (options.allowedTags || []).indexOf(tag) > -1 &&
       !options.allowVulnerableTags
     ) {
       console.warn(`\n\n⚠️ Your \`allowedTags\` option includes, \`${tag}\`, which is inherently\nvulnerable to XSS attacks. Please remove it from \`allowedTags\`.\nOr, to disable this warning, add the \`allowVulnerableTags\` option\nand ensure you are accounting for this risk.\n\n`);
@@ -251,7 +251,7 @@ function sanitizeHtml(html, options, _recursing) {
         }
       }
 
-      if ((options.allowedTags && options.allowedTags.indexOf(name) === -1) || (options.disallowedTagsMode === 'recursiveEscape' && !isEmptyObject(skipMap)) || (options.nestingLimit != null && depth >= options.nestingLimit)) {
+      if ((options.allowedTags !== false && (options.allowedTags || []).indexOf(name) === -1) || (options.disallowedTagsMode === 'recursiveEscape' && !isEmptyObject(skipMap)) || (options.nestingLimit != null && depth >= options.nestingLimit)) {
         skip = true;
         skipMap[depth] = true;
         if (options.disallowedTagsMode === 'discard') {

test/test.js

@@ -40,6 +40,26 @@ describe('sanitizeHtml', function() {
       allowedAttributes: false
     }), '<div><wiggly worms="ewww">hello</wiggly></div>');
   });
+  it('should not pass through any markup if allowedTags is set to undefined (falsy but not exactly false)', function() {
+    assert.equal(sanitizeHtml('<div><wiggly worms="ewww">hello</wiggly></div>', {
+      allowedTags: undefined
+    }), 'hello');
+  });
+  it('should not pass through any markup if allowedTags is set to 0 (falsy but not exactly false)', function() {
+    assert.equal(sanitizeHtml('<div><wiggly worms="ewww">hello</wiggly></div>', {
+      allowedTags: 0
+    }), 'hello');
+  });
+  it('should not pass through any markup if allowedTags is set to null (falsy but not exactly false)', function() {
+    assert.equal(sanitizeHtml('<div><wiggly worms="ewww">hello</wiggly></div>', {
+      allowedTags: null
+    }), 'hello');
+  });
+  it('should not pass through any markup if allowedTags is set to empty string (falsy but not exactly false)', function() {
+    assert.equal(sanitizeHtml('<div><wiggly worms="ewww">hello</wiggly></div>', {
+      allowedTags: ''
+    }), 'hello');
+  });
   it('should respect text nodes at top level', function() {
     assert.equal(sanitizeHtml('Blah blah blah<p>Whee!</p>'), 'Blah blah blah<p>Whee!</p>');
   });