Skip to content

Commit 12046d3

Browse files
martgilmartgil
authored
Merge branch 'master' into issue-5668-add-thunderbird-attachments-support
2 parents 8a36e6c + 9a50933 commit 12046d3

File tree

10 files changed

+55
-183
lines changed

10 files changed

+55
-183
lines changed

‎conf/tsconfig.content_scripts.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
"outDir": "../build/_/content_scripts",
1717
"baseUrl": "../extension",
1818
"paths": {
19-
"dompurify": ["types/purify.d.ts"],
19+
"dompurify": ["../node_modules/dompurify/dist/purify.cjs.d.ts"],
2020
"openpgp": ["../node_modules/openpgp/openpgp.d.ts"],
2121
"@openpgp/web-stream-tools": ["../node_modules/@openpgp/web-stream-tools/lib/index.d.ts"],
2222
"squire-rte": ["../node_modules/squire-rte/dist/types/Squire.d.ts"],

‎extension/chrome/elements/compose-modules/compose-render-module.ts‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,9 @@ export class ComposeRenderModule extends ViewModule<ComposeView> {
9898
const thread = await this.view.emailProvider.threadGet(this.view.threadId, 'metadata');
9999
const inReplyToMessage = thread.messages?.find(message => message.id === this.view.replyMsgId);
100100
if (inReplyToMessage) {
101-
this.view.replyParams.inReplyTo = inReplyToMessage.payload?.headers?.find(
102-
header => header.name === 'Message-Id' || header.name === 'Message-ID'
103-
)?.value;
101+
const msgId = inReplyToMessage.payload?.headers?.find(header => header.name === 'Message-Id' || header.name === 'Message-ID')?.value;
102+
const references = inReplyToMessage.payload?.headers?.find(header => header.name === 'References')?.value;
103+
this.setReplyHeaders(msgId, references);
104104
}
105105
this.view.replyParams.subject = `${this.responseMethod === 'reply' ? 'Re' : 'Fwd'}: ${this.view.replyParams.subject}`;
106106
if (this.view.useFullScreenSecureCompose) {
@@ -117,8 +117,7 @@ export class ComposeRenderModule extends ViewModule<ComposeView> {
117117
);
118118
if (this.view.quoteModule.messageToReplyOrForward) {
119119
const msgId = this.view.quoteModule.messageToReplyOrForward.headers['message-id'];
120-
this.view.sendBtnModule.additionalMsgHeaders['In-Reply-To'] = msgId;
121-
this.view.sendBtnModule.additionalMsgHeaders.References = this.view.quoteModule.messageToReplyOrForward.headers.references + ' ' + msgId;
120+
this.setReplyHeaders(msgId, this.view.quoteModule.messageToReplyOrForward.headers.references);
122121
if (this.view.replyPubkeyMismatch) {
123122
await this.renderReplyMsgAsReplyPubkeyMismatch();
124123
} else if (this.view.quoteModule.messageToReplyOrForward.isOnlySigned) {
@@ -273,6 +272,13 @@ export class ComposeRenderModule extends ViewModule<ComposeView> {
273272
}
274273
};
275274

275+
private setReplyHeaders = (msgId?: string, references?: string) => {
276+
if (msgId) {
277+
this.view.sendBtnModule.additionalMsgHeaders['In-Reply-To'] = msgId;
278+
this.view.sendBtnModule.additionalMsgHeaders.References = [references, msgId].filter(Boolean).join(' ');
279+
}
280+
};
281+
276282
private initComposeBoxStyles = () => {
277283
if (this.view.isReplyBox) {
278284
this.view.S.cached('body').addClass('reply_box');

‎extension/chrome/elements/compose-modules/compose-send-btn-module.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ export class ComposeSendBtnModule extends ViewModule<ComposeView> {
179179
return;
180180
}
181181
if ('src' in node) {
182-
const img: Element = node;
182+
const img = node as HTMLImageElement;
183183
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
184184
const src = img.getAttribute('src')!;
185185
const { mimeType, data } = this.parseInlineImageSrc(src);

‎extension/js/common/message-renderer.ts‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -83,28 +83,28 @@ export class MessageRenderer {
8383
const inlineCIDAttachments = new Set<Attachment>();
8484

8585
// Define the hook function for DOMPurify to process image elements after sanitizing attributes
86-
const processImageElements = (node: Element | null) => {
87-
// Ensure the node exists and has a 'src' attribute
88-
if (!node || !('src' in node)) return;
89-
const imageSrc = node.getAttribute('src');
86+
DOMPurify.addHook('afterSanitizeAttributes', currentNode => {
87+
// Ensure the node is an Element
88+
if (!(currentNode instanceof Element)) return;
89+
90+
// Check if the node has a 'src' attribute
91+
const imageSrc = currentNode.getAttribute('src');
9092
if (!imageSrc) return;
93+
9194
const matches = imageSrc.match(CID_PATTERN);
9295

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

98101
// Replace the src attribute with a base64 encoded string
99102
if (contentIdAttachment) {
100103
inlineCIDAttachments.add(contentIdAttachment);
101-
node.setAttribute('src', `data:${contentIdAttachment.type};base64,${contentIdAttachment.getData().toBase64Str()}`);
104+
currentNode.setAttribute('src', `data:${contentIdAttachment.type};base64,${contentIdAttachment.getData().toBase64Str()}`);
102105
}
103106
}
104-
};
105-
106-
// Add the DOMPurify hook
107-
DOMPurify.addHook('afterSanitizeAttributes', processImageElements);
107+
});
108108

109109
// Sanitize the HTML and remove the DOMPurify hooks
110110
const sanitizedHtml = Xss.htmlSanitize(html);

‎extension/js/common/platform/xss.ts‎

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -107,44 +107,56 @@ export class Xss {
107107
// used whenever untrusted remote content (eg html email) is rendered, but we still want to preserve html
108108
DOMPurify.removeAllHooks();
109109
DOMPurify.addHook('afterSanitizeAttributes', node => {
110-
if (!node) {
110+
// Ensure the node is an Element
111+
if (!(node instanceof Element)) {
111112
return;
112113
}
113-
if ('style' in node) {
114+
115+
// Handle style attributes
116+
if (node.hasAttribute('style')) {
114117
// mitigation rather than a fix, which will involve updating CSP, see https://github.com/FlowCrypt/flowcrypt-browser/issues/2648
115-
const style = (node as Element).getAttribute('style')?.toLowerCase();
118+
const style = node.getAttribute('style')?.toLowerCase();
116119
if (style && (style.includes('url(') || style.includes('@import'))) {
117-
(node as Element).removeAttribute('style'); // don't want any leaks through css url()
120+
node.removeAttribute('style'); // don't want any leaks through css url()
118121
}
119122
// strip css styles that could use to overlap with the extension UI
120123
if (style && Xss.FORBID_CSS_STYLE.test(style)) {
121124
const updatedStyle = style.replace(Xss.FORBID_CSS_STYLE, '');
122-
(node as HTMLElement).setAttribute('style', updatedStyle);
125+
node.setAttribute('style', updatedStyle);
123126
}
124127
}
125-
if ('src' in node) {
126-
const img = node as HTMLImageElement;
128+
129+
// Handle image attributes
130+
if (node.tagName === 'IMG') {
131+
const img = node as HTMLImageElement; // Narrow type to HTMLImageElement
127132
const src = img.getAttribute('src');
128133
if (imgHandling === 'IMG-DEL') {
129134
img.remove(); // just skip images
130135
} else if (!src) {
131136
img.remove(); // src that exists but is null is suspicious
132137
} else if (imgHandling === 'IMG-KEEP' && checkValidURL(src)) {
133138
// replace remote image with remote_image_container
134-
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>`;
139+
const remoteImgEl = `
140+
<div class="remote_image_container" data-src="${src}" data-test="remote-image-container">
141+
<span>Authenticity of this remote image cannot be verified.</span>
142+
</div>`;
135143
Xss.replaceElementDANGEROUSLY(img, remoteImgEl); // xss-safe-value
136144
}
137145
}
146+
147+
// Handle custom containers or CID-patterned src
138148
if ((node.classList.contains('remote_image_container') || CID_PATTERN.test(node.getAttribute('src') ?? '')) && imgHandling === 'IMG-TO-PLAIN-TEXT') {
139-
Xss.replaceElementDANGEROUSLY(node, node.getAttribute('data-src') ?? node.getAttribute('alt') ?? ''); // xss-safe-value
149+
const replacement = node.getAttribute('data-src') ?? node.getAttribute('alt') ?? '';
150+
Xss.replaceElementDANGEROUSLY(node, replacement); // xss-safe-value
140151
}
141-
if ('target' in node) {
142-
// open links in new window
143-
(node as Element).setAttribute('target', '_blank');
144-
// prevents https://www.owasp.org/index.php/Reverse_Tabnabbing
145-
(node as Element).setAttribute('rel', 'noopener noreferrer');
152+
153+
// Handle links (target and rel attributes)
154+
if (node.tagName === 'A') {
155+
node.setAttribute('target', '_blank'); // prevents https://www.owasp.org/index.php/Reverse_Tabnabbing
156+
node.setAttribute('rel', 'noopener noreferrer');
146157
}
147158
});
159+
148160
const cleanHtml = Xss.htmlSanitize(dirtyHtml, true);
149161
DOMPurify.removeAllHooks();
150162
return cleanHtml;

‎extension/types/purify.d.ts‎

Lines changed: 0 additions & 138 deletions
This file was deleted.

‎extension/types/trusted-types.d.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
11
interface Window {
22
trustedTypes: any;
3-
}
3+
}
4+
5+
interface TrustedHTML {}
6+
7+
interface TrustedTypePolicy {}

‎package-lock.json‎

Lines changed: 0 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
"@types/chai": "4.3.19",
1111
"@types/chai-as-promised": "7.1.8",
1212
"@types/chrome": "0.0.284",
13-
"@types/dompurify": "3.2.0",
1413
"@types/jquery": "3.5.32",
1514
"@types/mailparser": "3.4.5",
1615
"ava": "5.3.1",

‎tsconfig.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
"pdfjs": ["lib/pdf.min.mjs", "types/pdf.js.d.ts"],
2525
"squire-rte": ["../node_modules/squire-rte/dist/types/Squire.d.ts"],
2626
"@openpgp/web-stream-tools": ["../node_modules/@openpgp/web-stream-tools/lib/index.d.ts", "lib/streams/streams.js"],
27-
"dompurify": ["types/purify.d.ts", "lib/purify.js", "COMMENT"],
27+
"dompurify": ["../node_modules/dompurify/dist/purify.cjs.d.ts", "lib/purify.js", "COMMENT"],
2828
"fine-uploader": ["lib/fine-uploader.js", "COMMENT"],
2929
"clipboard": ["lib/clipboard.js", "COMMENT"],
3030
"filesize": ["lib/filesize.js", "COMMENT"],

0 commit comments

Comments
 (0)