Skip to content

Commit

Permalink
feat: use built in dompurify
Browse files Browse the repository at this point in the history
  • Loading branch information
ioanmo226 committed Nov 25, 2024
1 parent aa27485 commit 9377f53
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 178 deletions.
2 changes: 1 addition & 1 deletion conf/tsconfig.content_scripts.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"outDir": "../build/_/content_scripts",
"baseUrl": "../extension",
"paths": {
"dompurify": ["types/purify.d.ts"],
"dompurify": ["../node_modules/dompurify/dist/purify.cjs.d.ts"],
"openpgp": ["../node_modules/openpgp/openpgp.d.ts"],
"@openpgp/web-stream-tools": ["../node_modules/@openpgp/web-stream-tools/lib/index.d.ts"],
"squire-rte": ["../node_modules/squire-rte/dist/types/Squire.d.ts"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ export class ComposeSendBtnModule extends ViewModule<ComposeView> {
return;
}
if ('src' in node) {
const img: Element = node;
const img = node as HTMLImageElement;
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const src = img.getAttribute('src')!;
const { mimeType, data } = this.parseInlineImageSrc(src);
Expand Down
20 changes: 10 additions & 10 deletions extension/js/common/message-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,28 +83,28 @@ export class MessageRenderer {
const inlineCIDAttachments = new Set<Attachment>();

// Define the hook function for DOMPurify to process image elements after sanitizing attributes
const processImageElements = (node: Element | null) => {
// Ensure the node exists and has a 'src' attribute
if (!node || !('src' in node)) return;
const imageSrc = node.getAttribute('src');
DOMPurify.addHook('afterSanitizeAttributes', currentNode => {
// Ensure the node is an Element
if (!(currentNode instanceof Element)) return;

// Check if the node has a 'src' attribute
const imageSrc = currentNode.getAttribute('src');
if (!imageSrc) return;

const matches = imageSrc.match(CID_PATTERN);

// Check if the src attribute contains a CID
// If the src attribute contains a CID
if (matches?.[1]) {
const contentId = matches[1];
const contentIdAttachment = attachments.find(attachment => attachment.cid === `<${contentId}>`);

// Replace the src attribute with a base64 encoded string
if (contentIdAttachment) {
inlineCIDAttachments.add(contentIdAttachment);
node.setAttribute('src', `data:${contentIdAttachment.type};base64,${contentIdAttachment.getData().toBase64Str()}`);
currentNode.setAttribute('src', `data:${contentIdAttachment.type};base64,${contentIdAttachment.getData().toBase64Str()}`);
}
}
};

// Add the DOMPurify hook
DOMPurify.addHook('afterSanitizeAttributes', processImageElements);
});

// Sanitize the HTML and remove the DOMPurify hooks
const sanitizedHtml = Xss.htmlSanitize(html);
Expand Down
40 changes: 26 additions & 14 deletions extension/js/common/platform/xss.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,44 +107,56 @@ export class Xss {
// used whenever untrusted remote content (eg html email) is rendered, but we still want to preserve html
DOMPurify.removeAllHooks();
DOMPurify.addHook('afterSanitizeAttributes', node => {
if (!node) {
// Ensure the node is an Element
if (!(node instanceof Element)) {
return;
}
if ('style' in node) {

// Handle style attributes
if (node.hasAttribute('style')) {
// mitigation rather than a fix, which will involve updating CSP, see https://github.com/FlowCrypt/flowcrypt-browser/issues/2648
const style = (node as Element).getAttribute('style')?.toLowerCase();
const style = node.getAttribute('style')?.toLowerCase();
if (style && (style.includes('url(') || style.includes('@import'))) {
(node as Element).removeAttribute('style'); // don't want any leaks through css url()
node.removeAttribute('style'); // don't want any leaks through css url()
}
// strip css styles that could use to overlap with the extension UI
if (style && Xss.FORBID_CSS_STYLE.test(style)) {
const updatedStyle = style.replace(Xss.FORBID_CSS_STYLE, '');
(node as HTMLElement).setAttribute('style', updatedStyle);
node.setAttribute('style', updatedStyle);
}
}
if ('src' in node) {
const img = node as HTMLImageElement;

// Handle image attributes
if (node.tagName === 'IMG') {
const img = node as HTMLImageElement; // Narrow type to HTMLImageElement
const src = img.getAttribute('src');
if (imgHandling === 'IMG-DEL') {
img.remove(); // just skip images
} else if (!src) {
img.remove(); // src that exists but is null is suspicious
} else if (imgHandling === 'IMG-KEEP' && checkValidURL(src)) {
// replace remote image with remote_image_container
const remoteImgEl = `<div class="remote_image_container" data-src="${src}" data-test="remote-image-container"><span>Authenticity of this remote image cannot be verified.</span></div>`;
const remoteImgEl = `
<div class="remote_image_container" data-src="${src}" data-test="remote-image-container">
<span>Authenticity of this remote image cannot be verified.</span>
</div>`;
Xss.replaceElementDANGEROUSLY(img, remoteImgEl); // xss-safe-value
}
}

// Handle custom containers or CID-patterned src
if ((node.classList.contains('remote_image_container') || CID_PATTERN.test(node.getAttribute('src') ?? '')) && imgHandling === 'IMG-TO-PLAIN-TEXT') {
Xss.replaceElementDANGEROUSLY(node, node.getAttribute('data-src') ?? node.getAttribute('alt') ?? ''); // xss-safe-value
const replacement = node.getAttribute('data-src') ?? node.getAttribute('alt') ?? '';
Xss.replaceElementDANGEROUSLY(node, replacement); // xss-safe-value
}
if ('target' in node) {
// open links in new window
(node as Element).setAttribute('target', '_blank');
// prevents https://www.owasp.org/index.php/Reverse_Tabnabbing
(node as Element).setAttribute('rel', 'noopener noreferrer');

// Handle links (target and rel attributes)
if (node.tagName === 'A') {
node.setAttribute('target', '_blank'); // prevents https://www.owasp.org/index.php/Reverse_Tabnabbing
node.setAttribute('rel', 'noopener noreferrer');
}
});

const cleanHtml = Xss.htmlSanitize(dirtyHtml, true);
DOMPurify.removeAllHooks();
return cleanHtml;
Expand Down
138 changes: 0 additions & 138 deletions extension/types/purify.d.ts

This file was deleted.

6 changes: 5 additions & 1 deletion extension/types/trusted-types.d.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
interface Window {
trustedTypes: any;
}
}

interface TrustedHTML {}

interface TrustedTypePolicy {}
11 changes: 0 additions & 11 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"@types/chai": "4.3.19",
"@types/chai-as-promised": "7.1.8",
"@types/chrome": "0.0.284",
"@types/dompurify": "3.2.0",
"@types/jquery": "3.5.32",
"@types/mailparser": "3.4.5",
"ava": "5.3.1",
Expand Down
2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"pdfjs": ["lib/pdf.min.mjs", "types/pdf.js.d.ts"],
"squire-rte": ["../node_modules/squire-rte/dist/types/Squire.d.ts"],
"@openpgp/web-stream-tools": ["../node_modules/@openpgp/web-stream-tools/lib/index.d.ts", "lib/streams/streams.js"],
"dompurify": ["types/purify.d.ts", "lib/purify.js", "COMMENT"],
"dompurify": ["../node_modules/dompurify/dist/purify.cjs.d.ts", "lib/purify.js", "COMMENT"],
"fine-uploader": ["lib/fine-uploader.js", "COMMENT"],
"clipboard": ["lib/clipboard.js", "COMMENT"],
"filesize": ["lib/filesize.js", "COMMENT"],
Expand Down

0 comments on commit 9377f53

Please sign in to comment.