From 6f87d4ea71f68a751a46edb9cdfee99bf5ef9215 Mon Sep 17 00:00:00 2001 From: Eric Norris Date: Sun, 29 Nov 2020 14:50:48 -0500 Subject: [PATCH 1/3] feat: add option to specify only disallowed tags --- .eslintrc.js | 2 +- src/states.ts | 20 ++++++++++-- src/striptags.ts | 3 +- tests/states.test.ts | 62 +++++++++++++++++++++++++++-------- tests/striptags.test.ts | 71 ++++++++++++++++++++++++++++++++--------- 5 files changed, 124 insertions(+), 34 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 4485125..cb412b6 100644 --- a/.eslintrc.js +++ b/.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"], }, }; diff --git a/src/states.ts b/src/states.ts index ef51a13..5299757 100644 --- a/src/states.ts +++ b/src/states.ts @@ -17,7 +17,9 @@ const ENCODED_TAG_START = "<"; const ENCODED_TAG_END = ">"; export interface StateMachineOptions { - readonly allowedTags: Set; + readonly allowedTags?: Set; + readonly disallowedTags?: Set; + 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 = ( diff --git a/src/striptags.ts b/src/striptags.ts index ae21e80..8d24d1c 100644 --- a/src/striptags.ts +++ b/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(), tagReplacementText: "", encodePlaintextTagDelimiters: true, }; diff --git a/tests/states.test.ts b/tests/states.test.ts index d1ad457..cdfa589 100644 --- a/tests/states.test.ts +++ b/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([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 = "<" + text; + const want = `<${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); diff --git a/tests/striptags.test.ts b/tests/striptags.test.ts index 0146247..fd6c7ac 100644 --- a/tests/striptags.test.ts +++ b/tests/striptags.test.ts @@ -1,28 +1,69 @@ -import { StateMachine, striptags } from "../src/striptags"; +import { StateMachineOptions, StateMachine, striptags } from "../src/striptags"; + +const OptionsWithAllowedTags: Partial = { + allowedTags: new Set(["atag"]), + tagReplacementText: " ", +}; + +const OptionsWithDisallowedTags: Partial = { + disallowedTags: new Set(["atag"]), + tagReplacementText: " ", +}; + +const ExampleText = `some textmore text`; + +const WantWhenUsingDefault = "some textmore text"; +const WantWhenUsingAllowedTags = `some text more text `; +const WantWhenUsingDisallowedTags = " some textmore text "; 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 some text"; + const got = ExampleText.split(/(?= )/g) + .map((partial) => machine.consume(partial)) + .join(""); - const got = - machine.consume("a string 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 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 some text", { - allowedTags: new Set(["atag"]), - }); + it("disallowed tags sanity check", () => { + const got = striptags(ExampleText, OptionsWithDisallowedTags); - expect(got).toEqual(want); + expect(got).toEqual(WantWhenUsingDisallowedTags); }); }); From c1028435465253c40ebc06e69fa660de42a517b3 Mon Sep 17 00:00:00 2001 From: Eric Norris Date: Sun, 29 Nov 2020 15:32:46 -0500 Subject: [PATCH 2/3] docs: update README.md --- README.md | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index f860449..119003b 100644 --- a/README.md +++ b/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: ``. + +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` -* `allowedTags?: Set` 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`** + +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`** + +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` From f3dc241c7f3ac93040c7f8a658036fb348ee04c3 Mon Sep 17 00:00:00 2001 From: Eric Norris Date: Sun, 29 Nov 2020 15:38:54 -0500 Subject: [PATCH 3/3] chore: bump alpha version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 4105827..5ab161c 100644 --- a/package.json +++ b/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",