Merge pull request #61 from ericnorris/disallowed-tags-option

Closes #32.

.eslintrc.js

@@ -15,7 +15,7 @@ module.exports = {
         indent: ["error", 4],
         "linebreak-style": ["error", "unix"],
         "prettier/prettier": "error",
-        quotes: ["error", "double", { avoidEscape: true }],
+        quotes: ["error", "double", { avoidEscape: true, allowTemplateLiterals: true }],
         semi: ["error", "always"],
     },
 };

README.md

@@ -76,8 +76,34 @@ Outputs:
 some text with and more text
 ```
 
+## Safety
+
+`striptags` is safe to use by default; the output is guaranteed to be free of potential XSS vectors if used as text within a tag. **Specifying either `allowedTags` or `disallowedTags` in the options argument removes this guarantee**, however. For example, a malicious user may achieve XSS via an attribute in an allowed tag: `<img onload="alert(1);">`.
+
+In addition, `striptags` will automatically HTML encode `<` and `>` characters followed by whitespace. While most browsers tested treat `<` or `>` followed by whitespace as a non-tag string, it is safer to escape the characters. You may change this behavior via the `encodePlaintextTagDelimiters` option described below.
+
 ## `Partial<StateMachineOptions>`
 
-* `allowedTags?: Set<string>` a set containing a list of tag names to allow (e.g. `new Set(["tagname"])`), default: `new Set([])`.
-* `tagReplacementText?: string` a string to use as replacement text when a tag is found and not allowed, default: `""`.
-* `encodePlaintextTagDelimiters?: boolean` true if `<` and `>` characters immediately followed by whitespace should be HTML encoded, default: `true`. This is safe to set to `false` if the output is expected to be used only as plaintext.
+**`allowedTags?: Set<string>`**
+
+A set containing a list of tag names to allow (e.g. `new Set(["tagname"])`). Tags not in this list will be removed. This option takes precedence over the `disallowedTags` option.
+
+Default: `undefined`
+
+**`disallowedTags?: Set<string>`**
+
+A set containing a list of tag names to disallow ((e.g. `new Set(["tagname"])`). Tags not in this list will be allowed. Ignored if `allowedTags` is set.
+
+Default: `undefined`
+
+**`tagReplacementText?: string`**
+
+A string to use as replacement text when a tag is found and not allowed.
+
+Default: `""`
+
+**`encodePlaintextTagDelimiters?: boolean`**
+
+Setting this option to true will cause `<` and `>` characters immediately followed by whitespace to be HTML encoded. This is safe to set to `false` if the output is expected to be used only as plaintext (i.e. it will not be displayed alongside other HTML).
+
+Default: `true`

package.json

@@ -9,7 +9,7 @@
     },
     "homepage": "https://github.com/ericnorris/striptags",
     "bugs": "https://github.com/ericnorris/striptags/issues",
-    "version": "4.0.0-alpha.0",
+    "version": "4.0.0-alpha.1",
     "keywords": [
         "striptags",
         "strip_tags",

src/states.ts

@@ -17,7 +17,9 @@ const ENCODED_TAG_START = "<";
 const ENCODED_TAG_END = ">";
 
 export interface StateMachineOptions {
-    readonly allowedTags: Set<string>;
+    readonly allowedTags?: Set<string>;
+    readonly disallowedTags?: Set<string>;
+
     readonly tagReplacementText: string;
     readonly encodePlaintextTagDelimiters: boolean;
 }
@@ -84,7 +86,7 @@ export class InTagNameState implements State {
         }
 
         if (isSpace(character)) {
-            if (this.options.allowedTags.has(this.nameBuffer.toLowerCase())) {
+            if (this.isNameBufferAnAllowedTag()) {
                 transition(new InTagState(TagMode.Allowed, this.options));
 
                 return TAG_START + (this.isClosingTag ? "/" : "") + this.nameBuffer + character;
@@ -102,7 +104,7 @@ export class InTagNameState implements State {
         if (character == TAG_END) {
             transition(new InPlaintextState(this.options));
 
-            if (this.options.allowedTags.has(this.nameBuffer.toLowerCase())) {
+            if (this.isNameBufferAnAllowedTag()) {
                 return TAG_START + (this.isClosingTag ? "/" : "") + this.nameBuffer + character;
             } else {
                 return this.options.tagReplacementText;
@@ -119,6 +121,18 @@ export class InTagNameState implements State {
 
         return "";
     }
+
+    private isNameBufferAnAllowedTag(): boolean {
+        const tagName = this.nameBuffer.toLowerCase();
+
+        if (this.options.allowedTags) {
+            return this.options.allowedTags.has(tagName);
+        } else if (this.options.disallowedTags) {
+            return !this.options.disallowedTags.has(tagName);
+        } else {
+            return false;
+        }
+    }
 }
 
 type InTagStateTransitionFunction<T extends TagMode> = (

src/striptags.ts

@@ -1,7 +1,8 @@
 import { StateMachineOptions, State, StateTransitionFunction, InPlaintextState } from "./states";
 
+export { StateMachineOptions } from "./states";
+
 export const DefaultStateMachineOptions: StateMachineOptions = {
-    allowedTags: new Set<string>(),
     tagReplacementText: "",
     encodePlaintextTagDelimiters: true,
 };

tests/states.test.ts

@@ -10,23 +10,31 @@ import {
 } from "../src/states";
 
 const AllowedTagName = "allowed";
+const DisallowedTagName = "disallowed";
+const ImplicitlyAllowedTagName = "notdisallowed";
 const TagReplacementText = "~replaced~";
 
-const DefaultOptions = {
-    allowedTags: new Set<string>([AllowedTagName]),
+const DefaultTestOptions = {
+    allowedTags: new Set([AllowedTagName]),
     tagReplacementText: TagReplacementText,
 };
 
 const OptionsWithEncodingEnabled: StateMachineOptions = {
-    ...DefaultOptions,
+    ...DefaultTestOptions,
     encodePlaintextTagDelimiters: true,
 };
 
 const OptionsWithEncodingDisabled: StateMachineOptions = {
-    ...DefaultOptions,
+    ...DefaultTestOptions,
     encodePlaintextTagDelimiters: false,
 };
 
+const OptionsWithDisallowedTags: StateMachineOptions = {
+    disallowedTags: new Set([DisallowedTagName]),
+    tagReplacementText: TagReplacementText,
+    encodePlaintextTagDelimiters: true,
+};
+
 function consumeStringUntilTransitionOrEOF(start: State, text: string): [string, State] {
     let currentState = start;
     let outputBuffer = "";
@@ -87,7 +95,7 @@ describe("InTagNameState", () => {
         const start = new InTagNameState(OptionsWithEncodingEnabled);
 
         const text = " ";
-        const want = "&lt;" + text;
+        const want = `&lt;${text}`;
 
         const [got, endState] = consumeStringUntilTransitionOrEOF(start, text);
 
@@ -99,7 +107,7 @@ describe("InTagNameState", () => {
         const start = new InTagNameState(OptionsWithEncodingDisabled);
 
         const text = " ";
-        const want = "<" + text;
+        const want = `<${text}`;
 
         const [got, endState] = consumeStringUntilTransitionOrEOF(start, " ");
 
@@ -110,8 +118,8 @@ describe("InTagNameState", () => {
     it("should transition to InTagState w/ allowed mode upon seeing a space", () => {
         const start = new InTagNameState(OptionsWithEncodingEnabled);
 
-        const text = AllowedTagName + " ";
-        const want = "<" + text;
+        const text = `${AllowedTagName} `;
+        const want = `<${text}`;
 
         const [got, endState] = consumeStringUntilTransitionOrEOF(start, text);
 
@@ -123,8 +131,8 @@ describe("InTagNameState", () => {
     it("should transition to InTagState w/ allowed mode upon seeing a space after a closing tag name", () => {
         const start = new InTagNameState(OptionsWithEncodingEnabled);
 
-        const text = "/" + AllowedTagName + " ";
-        const want = "<" + text;
+        const text = `/${AllowedTagName} `;
+        const want = `<${text}`;
 
         const [got, endState] = consumeStringUntilTransitionOrEOF(start, text);
 
@@ -149,8 +157,8 @@ describe("InTagNameState", () => {
     it("should allow tags with no attributes", () => {
         const start = new InTagNameState(OptionsWithEncodingEnabled);
 
-        const text = AllowedTagName + ">";
-        const want = "<" + text;
+        const text = `${AllowedTagName}>`;
+        const want = `<${text}`;
 
         const [got, endState] = consumeStringUntilTransitionOrEOF(start, text);
 
@@ -161,8 +169,8 @@ describe("InTagNameState", () => {
     it("should allow closing tags with no attributes", () => {
         const start = new InTagNameState(OptionsWithEncodingEnabled);
 
-        const text = "/" + AllowedTagName + ">";
-        const want = "<" + text;
+        const text = `/${AllowedTagName}>`;
+        const want = `<${text}`;
 
         const [got, endState] = consumeStringUntilTransitionOrEOF(start, text);
 
@@ -182,6 +190,32 @@ describe("InTagNameState", () => {
         expect(endState).toBeInstanceOf(InPlaintextState);
     });
 
+    it("should disallow tags in a disallowedTags set", () => {
+        const start = new InTagNameState(OptionsWithDisallowedTags);
+
+        const text = `${DisallowedTagName} `;
+        const want = TagReplacementText;
+
+        const [got, endState] = consumeStringUntilTransitionOrEOF(start, text);
+
+        expect(got).toEqual(want);
+        expect(endState).toBeInstanceOf(InTagState);
+        expect(endState).toHaveProperty("mode", TagMode.Disallowed);
+    });
+
+    it("should allow tags not in a disallowedTags set", () => {
+        const start = new InTagNameState(OptionsWithDisallowedTags);
+
+        const text = `${ImplicitlyAllowedTagName} `;
+        const want = `<${text}`;
+
+        const [got, endState] = consumeStringUntilTransitionOrEOF(start, text);
+
+        expect(got).toEqual(want);
+        expect(endState).toBeInstanceOf(InTagState);
+        expect(endState).toHaveProperty("mode", TagMode.Allowed);
+    });
+
     it("should transition to InCommentState for comments", () => {
         const start = new InTagNameState(OptionsWithEncodingEnabled);
 

tests/striptags.test.ts

@@ -1,28 +1,69 @@
-import { StateMachine, striptags } from "../src/striptags";
+import { StateMachineOptions, StateMachine, striptags } from "../src/striptags";
+
+const OptionsWithAllowedTags: Partial<StateMachineOptions> = {
+    allowedTags: new Set(["atag"]),
+    tagReplacementText: " ",
+};
+
+const OptionsWithDisallowedTags: Partial<StateMachineOptions> = {
+    disallowedTags: new Set(["atag"]),
+    tagReplacementText: " ",
+};
+
+const ExampleText = `<atag someattr="value">some text<btag>more text</btag></atag>`;
+
+const WantWhenUsingDefault = "some textmore text";
+const WantWhenUsingAllowedTags = `<atag someattr="value">some text more text </atag>`;
+const WantWhenUsingDisallowedTags = " some text<btag>more text</btag> ";
 
 describe("StateMachine", () => {
-    it("sanity check", () => {
-        const machine = new StateMachine({ allowedTags: new Set(["atag"]) });
+    it("defaults sanity check", () => {
+        const machine = new StateMachine();
+
+        const got = ExampleText.split(/(?= )/g)
+            .map((partial) => machine.consume(partial))
+            .join("");
+
+        expect(got).toEqual(WantWhenUsingDefault);
+    });
+
+    it("allowed tags sanity check", () => {
+        const machine = new StateMachine(OptionsWithAllowedTags);
 
-        const want = "a string </atag some attr>  some text";
+        const got = ExampleText.split(/(?= )/g)
+            .map((partial) => machine.consume(partial))
+            .join("");
 
-        const got =
-            machine.consume("a string </a") +
-            machine.consume("tag some attr> <a") +
-            machine.consume("nothertag> some text");
+        expect(got).toEqual(WantWhenUsingAllowedTags);
+    });
+
+    it("disallowed tags sanity check", () => {
+        const machine = new StateMachine(OptionsWithDisallowedTags);
 
-        expect(got).toEqual(want);
+        const got = ExampleText.split(/(?= )/g)
+            .map((partial) => machine.consume(partial))
+            .join("");
+
+        expect(got).toEqual(WantWhenUsingDisallowedTags);
     });
 });
 
 describe("striptags", () => {
-    it("sanity check", () => {
-        const want = "a string </atag some attr>  some text";
+    it("defaults sanity check", () => {
+        const got = striptags(ExampleText);
+
+        expect(got).toEqual(WantWhenUsingDefault);
+    });
+
+    it("allowed tags sanity check", () => {
+        const got = striptags(ExampleText, OptionsWithAllowedTags);
+
+        expect(got).toEqual(WantWhenUsingAllowedTags);
+    });
 
-        const got = striptags("a string </atag some attr> <anothertag> some text", {
-            allowedTags: new Set(["atag"]),
-        });
+    it("disallowed tags sanity check", () => {
+        const got = striptags(ExampleText, OptionsWithDisallowedTags);
 
-        expect(got).toEqual(want);
+        expect(got).toEqual(WantWhenUsingDisallowedTags);
     });
 });