Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -807,9 +807,13 @@ export class ComposeRecipientsModule extends ViewModule<ComposeView> {
ulHtml += `<li class="select_contact" email="${Xss.escape(contact.email.replace(/<\/?b>/g, ''))}">`;
if (contact.pgpLoading) {
ulHtml += '<img class="loading-icon" data-test="pgp-loading-icon" src="/img/svgs/spinner-green-small.svg" />';
const emailForSelector = contact.email
.replace(/<\/?b>/g, '')
.replace(/\\/g, '\\\\')
.replace(/"/g, '\\"');
contact.pgpLoading
.then(hasPgp => {
Xss.replaceElementDANGEROUSLY($(`[email="${contact.email}"] .loading-icon`)[0], this.getPgpIconHtml(hasPgp)); // xss-direct
Xss.replaceElementDANGEROUSLY($(`[email="${emailForSelector}"] .loading-icon`)[0], this.getPgpIconHtml(hasPgp)); // xss-direct
})
.catch(() => {
this.failedLookupEmails.push(contact.email);
Expand Down
2 changes: 1 addition & 1 deletion extension/js/common/browser/ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ export class Ui {
},
confirmWithCheckbox: async (label: string, html = ''): Promise<boolean> => {
const userResponsePromise = Ui.swal().fire({
html,
html: Xss.htmlSanitize(html),
input: 'checkbox',
inputPlaceholder: label,
allowOutsideClick: false,
Expand Down
2 changes: 1 addition & 1 deletion extension/js/common/inject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ export class Injector {
if (this.webmailName === 'gmail') {
// eslint-disable-next-line local-rules/standard-loops
$('.' + (window as unknown as ContentScriptWindow).reloadable_class).each((i, reloadableEl) => {
$(reloadableEl).replaceWith($(reloadableEl)[0].outerHTML); // xss-reinsert - inserting code that was already present should not be dangerous
$(reloadableEl).replaceWith($(reloadableEl)[0].outerHTML); // xss-reinsert
});
} else {
window.location.reload();
Expand Down
15 changes: 9 additions & 6 deletions extension/js/common/message-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ export class MessageRenderer {
const fromString = GmailParser.findHeader(fullMsg, 'from');
const from = fromString ? Str.parseEmail(fromString) : undefined;
const fromEmail = from?.email ?? '';
const fromHtml = from?.name ? `<b>${Xss.escape(from.name)}</b> &lt;${fromEmail}&gt;` : fromEmail;
const fromHtml = from?.name ? `<b>${Xss.escape(from.name)}</b> &lt;${Xss.escape(fromEmail)}&gt;` : Xss.escape(fromEmail);
/* eslint-disable @typescript-eslint/no-non-null-assertion */
const ccString = GmailParser.findHeader(fullMsg, 'cc')
? `Cc: <span data-test="print-cc">${Xss.escape(GmailParser.findHeader(fullMsg, 'cc')!)}</span><br/>`
Expand All @@ -518,7 +518,7 @@ export class MessageRenderer {
<span data-test="print-from">From: ${fromHtml}</span>
</div>
<div class="float-right">
<span data-test="print-date">${sentDateStr}</span>
<span data-test="print-date">${Xss.escape(sentDateStr ?? '')}</span>
</div>
</div>
<span data-test="print-to">To: ${Xss.escape(GmailParser.findHeader(fullMsg, 'to') ?? '')}</span><br/>
Expand Down Expand Up @@ -780,7 +780,7 @@ export class MessageRenderer {
if (fallbackToPlainText) {
renderModule.renderAsRegularContent(Str.with(encryptedData));
} else {
renderModule.renderErr(Lang.pgpBlock.badFormat + '\n\n' + result.error.message, Str.with(encryptedData));
renderModule.renderErr(Lang.pgpBlock.badFormat + '\n\n' + Xss.escape(result.error.message), Str.with(encryptedData));
}
} else if (result.longids.needPassphrase.length) {
renderModule.renderPassphraseNeeded(result.longids.needPassphrase);
Expand All @@ -799,16 +799,19 @@ export class MessageRenderer {
renderModule.renderErr(Lang.pgpBlock.pwdMsgAskSenderUsePubkey, undefined);
} else if (result.error.type === DecryptErrTypes.noMdc) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
renderModule.renderErr(result.error.message, result.content!.toUtfStr()); // missing mdc - only render the result after user confirmation
renderModule.renderErr(Xss.escape(result.error.message), result.content!.toUtfStr()); // missing mdc - only render the result after user confirmation
} else if (result.error) {
renderModule.renderErr(`${Lang.pgpBlock.cantOpen}\n\n<em>${result.error.type}: ${result.error.message}</em>`, Str.with(encryptedData));
renderModule.renderErr(
`${Lang.pgpBlock.cantOpen}\n\n<em>${Xss.escape(result.error.type)}: ${Xss.escape(result.error.message)}</em>`,
Str.with(encryptedData)
);
} else {
// should generally not happen
renderModule.renderErr(
Lang.pgpBlock.cantOpen +
Lang.general.writeMeToFixIt(await isCustomerUrlFesUsed(this.acctEmail)) +
'\n\nDiagnostic info: "' +
JSON.stringify(result) +
Xss.escape(JSON.stringify(result)) +
'"',
Str.with(encryptedData)
);
Expand Down
78 changes: 70 additions & 8 deletions extension/js/common/platform/xss.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,34 @@ export class Xss {
'col',
];
private static ADD_ATTR = ['email', 'page', 'addurltext', 'longid', 'index', 'target', 'fingerprint', 'cryptup-data'];
private static FORBID_ATTR = ['background', 'srcset'];
private static FORBID_ATTR = ['background', 'srcset', 'ping'];
private static FORBID_TAGS = [
'script',
'noscript',
'style',
'link',
'meta',
'base',
'title',
'head',
'html',
'body',
'template',
'iframe',
'object',
'embed',
'math',
'form',
'textarea',
'menu',
'dialog',
'video',
'audio',
'source',
'canvas',
];
private static HREF_REGEX_CACHE: RegExp | undefined;
private static EMOJI_REGEX = /(?![*#0-9]+)[\p{Emoji}\p{Emoji_Modifier}\p{Emoji_Component}\p{Emoji_Modifier_Base}\p{Emoji_Presentation}]/gu;
/* eslint-disable @typescript-eslint/naming-convention */
private static readonly ALLOWED_EMAIL_CSS_PROPERTIES = new Set<string>([
// Colors
'color',
Expand Down Expand Up @@ -120,7 +144,7 @@ export class Xss {
'direction',
'unicode-bidi',
]);
/* eslint-enable @typescript-eslint/naming-convention */
private static readonly DANGEROUS_CSS_PROPERTIES = new Set<string>(['z-index', 'pointer-events', 'transform', 'filter', 'clip-path', 'clip']); // xss-none

public static sanitizeRender = (selector: string | HTMLElement | JQuery, dirtyHtml: string) => {
// browser-only (not on node)
Expand Down Expand Up @@ -151,14 +175,34 @@ export class Xss {
*/
public static htmlSanitize = (dirtyHtml: string, tagCheck = false): string => {
Xss.throwIfNotSupported();
const purgeStyleHook = (node: Node) => {
if (!(node instanceof Element)) {
return;
}
if (node.hasAttribute('style')) {
const style = Xss.purgeDangerousCss(node.getAttribute('style') || ''); // xss-none
if (style) {
node.setAttribute('style', style);
} else {
node.removeAttribute('style');
}
}
if (node.tagName === 'A' && (node.getAttribute('target') || '').toLowerCase().includes('_blank')) {
node.setAttribute('rel', 'noopener noreferrer');
}
};
DOMPurify.addHook('afterSanitizeAttributes', purgeStyleHook);
/* eslint-disable @typescript-eslint/naming-convention */
return DOMPurify.sanitize(dirtyHtml, {
const cleanHtml = DOMPurify.sanitize(dirtyHtml, {
ADD_ATTR: Xss.ADD_ATTR,
FORBID_ATTR: Xss.FORBID_ATTR,
FORBID_TAGS: Xss.FORBID_TAGS,
...(tagCheck && { ALLOWED_TAGS: Xss.ALLOWED_HTML_TAGS }),
ALLOWED_URI_REGEXP: Xss.sanitizeHrefRegexp(),
});
/* eslint-enable @typescript-eslint/naming-convention */
DOMPurify.removeHook('afterSanitizeAttributes', purgeStyleHook);
return cleanHtml;
};

/**
Expand Down Expand Up @@ -330,13 +374,13 @@ export class Xss {
};

// prettier-ignore
public static replaceElementDANGEROUSLY = (el: Element, safeHtml: string) => { // xss-dangerous-function - must pass a sanitized value
el.outerHTML = safeHtml; // xss-dangerous-function - must pass a sanitized value
public static replaceElementDANGEROUSLY = (el: Element, safeHtml: string) => { // xss-dangerous-function
el.outerHTML = safeHtml; // xss-dangerous-function
};

// prettier-ignore
public static setElementContentDANGEROUSLY = (el: Element, safeHtml: string) => { // xss-dangerous-function - must pass a sanitized value
el.innerHTML = safeHtml; // xss-dangerous-function - must pass a sanitized value
public static setElementContentDANGEROUSLY = (el: Element, safeHtml: string) => { // xss-dangerous-function
el.innerHTML = safeHtml; // xss-dangerous-function
};

private static throwIfNotSupported = () => {
Expand Down Expand Up @@ -365,6 +409,24 @@ export class Xss {
return style.cssText;
};

// prettier-ignore
private static purgeDangerousCss = (css: string): string => { // xss-none
if (!css || typeof document === 'undefined') {
return css;
}
const style = document.createElement('span').style;
style.cssText = css;
for (const property of Array.from(style)) {
const lower = property.toLowerCase();
if (lower === 'position' && ['fixed', 'absolute', 'sticky'].includes(style.getPropertyValue(property).trim().toLowerCase())) {
Comment thread
martgil marked this conversation as resolved.
Outdated
style.removeProperty(property);
} else if (this.DANGEROUS_CSS_PROPERTIES.has(lower)) { // xss-none
style.removeProperty(property);
}
}
return style.cssText;
};

/**
* allow href links that have same origin as our extension + cid + inline image
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ export const contentScriptSetupIfVacant = async (webmailSpecific: WebmailSpecifi
$('.' + win.destroyable_class).remove();
// eslint-disable-next-line local-rules/standard-loops
$('.' + win.reloadable_class).each((i, reloadableEl) => {
$(reloadableEl).replaceWith($(reloadableEl)[0].outerHTML); // xss-reinsert - inserting code that was already present should not be dangerous
$(reloadableEl).replaceWith($(reloadableEl)[0].outerHTML); // xss-reinsert
});
wasDestroyed = true;
})();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export class GmailElementReplacer extends WebmailElementReplacer {
let currentEmailContainer = $(emailContainer);
if (!this.isPlainTextOrHtml(blocksFromEmailContainer)) {
const { renderedXssSafe: renderedFromEmailContainerXssSafe } = this.messageRenderer.renderMsg({ blocks: blocksFromEmailContainer }, false); // xss-safe-value
currentEmailContainer = GmailLoaderContext.updateMsgBodyEl_DANGEROUSLY(emailContainer, 'set', renderedFromEmailContainerXssSafe); // xss-safe-factory: replace_blocks is XSS safe
currentEmailContainer = GmailLoaderContext.updateMsgBodyEl_DANGEROUSLY(emailContainer, 'set', renderedFromEmailContainerXssSafe); // xss-safe-factory
}

let blocks: MsgBlock[] = [];
Expand Down Expand Up @@ -238,7 +238,7 @@ export class GmailElementReplacer extends WebmailElementReplacer {
if (this.debug) {
console.debug('replaceArmoredBlocks() for of emailsContainingPgpBlock -> emailContainer replacing');
}
GmailLoaderContext.updateMsgBodyEl_DANGEROUSLY(currentEmailContainer, 'set', renderedXssSafe); // xss-safe-factory: replace_blocks is XSS safe
GmailLoaderContext.updateMsgBodyEl_DANGEROUSLY(currentEmailContainer, 'set', renderedXssSafe); // xss-safe-factory
if (this.debug) {
console.debug('replaceArmoredBlocks() for of emailsContainingPgpBlock -> emailContainer replaced');
}
Expand Down Expand Up @@ -946,7 +946,7 @@ export class GmailElementReplacer extends WebmailElementReplacer {
$(this.sel.draftsList).append(offlineDraftsContainer); // xss-safe-factory
for (const draftId of draftIdsSortedByTimestamp) {
const draft = offlineComposeDrafts[draftId];
const draftLink = $(`<a href>${new Date(draft.timestamp).toLocaleString()}</a>`);
const draftLink = $(`<a href>${Xss.escape(new Date(draft.timestamp).toLocaleString())}</a>`);
draftLink.on('click', event => {
event.preventDefault();
this.injector.openComposeWin(draftId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export class GmailLoaderContext implements LoaderContextInterface {
} else if (method === 'append') {
if (replace) {
const parent = msgBody.parent();
const existingHtml = msgBody.html() || ''; // xss-direct - preserving existing Gmail-rendered content
const existingHtml = msgBody.html() || ''; // xss-direct
msgBody.replaceWith(this.wrapMsgBodyEl(existingHtml + newHtmlContent_MUST_BE_XSS_SAFE)); // xss-safe-value
this.ensureHasParentNode(msgBody); // Gmail is using msgBody.parentNode (#2271)
return parent.find('.message_inner_body'); // need to return new selector - old element was replaced
Expand Down
Loading