Skip to content

Commit

Permalink
fix: Unescape HTML entities for alternate DOMPurify check (#2630)
Browse files Browse the repository at this point in the history
* fix: Unescape quotations before DOMPurify check

* fix: Handle nbsp

* feat: Log unclean HTML for future troubleshooting

* fix: Ensure original error is also logged
  • Loading branch information
DafyddLlyr authored Jan 5, 2024
1 parent 651721a commit 59d016a
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 2 deletions.
2 changes: 1 addition & 1 deletion api.planx.uk/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ const insertFlow = async (
return { id };
} catch (error) {
throw Error(
`User ${creatorId} failed to insert flow to teamId ${teamId}. Please check permissions.`,
`User ${creatorId} failed to insert flow to teamId ${teamId}. Please check permissions. Error: ${error}`,
);
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ describe("isCleanHTML() helper function", () => {
`<h2>Subheading</h2><p>This is a paragraph under the subheading.</p>`,
`<div><h1>Main Title</h1><p>This is a <b>nested</b> paragraph with a <i>nested</i> element.</p></div>`,
`<div><h1>Content with image</h1><img src="dogs.png" alt="Dog photo"></div>`,
`<div><p>Content&nbsp;with HTML entitie&apos;s&lt;&gt;&quot;&amp;</p></div>`,
];

for (const example of cleanHTML) {
Expand Down
31 changes: 30 additions & 1 deletion api.planx.uk/modules/webhooks/service/validateInput/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { isObject } from "lodash";
import { JSDOM } from "jsdom";
import createDOMPurify from "dompurify";
import { userContext } from "../../../auth/middleware";

// Setup JSDOM and DOMPurify
const window = new JSDOM("").window;
Expand Down Expand Up @@ -39,7 +40,35 @@ export const isCleanHTML = (input: unknown): boolean => {
if (typeof input !== "string") return true;

const cleanHTML = DOMPurify.sanitize(input, { ADD_ATTR: ["target"] });

// DOMPurify has not removed any attributes or values
const isClean = cleanHTML.length === input.length;
const isClean =
cleanHTML.length === input.length ||
unescapeHTML(cleanHTML).length === unescapeHTML(input).length;

if (!isClean) logUncleanHTMLError(input, cleanHTML);

return isClean;
};

/**
* Explicity log error when unsafe HTML is encountered
* This is very likely a content / sanitation error as opposed to a security issue
* Logging this should help us identify and resolve these
*/
const logUncleanHTMLError = (input: string, cleanHTML: string) => {
const userId = userContext.getStore()?.user.sub;

console.error({
message: `Warning: Unclean HTML submitted!`,
userId,
input,
cleanHTML,
});
};

const unescapeHTML = (input: string): string =>
input
.replace(/&quot;/gi, '"')
.replace(/&apos;/gi, "'")
.replace(/&nbsp;/gi, " ");

0 comments on commit 59d016a

Please sign in to comment.