Version bump main: 9.60.0 → 9.61.0

.github/workflows/build-and-deploy.yml

@@ -26,7 +26,7 @@ jobs:
               run: npm run-script build:ci
 
             - name: Install Firefox
-              uses: browser-actions/setup-firefox@latest
+              uses: browser-actions/setup-firefox@0bc507ddf224827e3b1af68e014d5e42ab93e795 # v1.7.2
 
             - name: Generate code coverage report
               uses: GabrielBB/xvfb-action@b706e4e27b14669b486812790492dc50ca16b465 # v1.7

.github/workflows/build-and-test.yml

@@ -37,7 +37,7 @@ jobs:
               run: yarn
 
             - name: Install Chrome
-              uses: browser-actions/setup-chrome@latest
+              uses: browser-actions/setup-chrome@48ad923757ca74d66703209fe939badbdf80f2f4 # v2.2.0
 
             - name: Test with Chrome
               uses: GabrielBB/xvfb-action@b706e4e27b14669b486812790492dc50ca16b465 # v1.7
@@ -60,7 +60,7 @@ jobs:
               run: yarn
 
             - name: Install Firefox
-              uses: browser-actions/setup-firefox@latest
+              uses: browser-actions/setup-firefox@0bc507ddf224827e3b1af68e014d5e42ab93e795 # v1.7.2
 
             - name: Test with Firefox
               uses: GabrielBB/xvfb-action@b706e4e27b14669b486812790492dc50ca16b465 # v1.7

packages/roosterjs-content-model-api/lib/publicApi/link/insertLink.ts

@@ -1,5 +1,4 @@
 import { adjustTrailingSpaceSelection } from '../../modelApi/selection/adjustTrailingSpaceSelection';
-import { checkXss } from '../utils/checkXss';
 import { matchLink } from '../../modelApi/link/matchLink';
 import {
     addLink,
@@ -8,6 +7,7 @@ import {
     createContentModelDocument,
     createText,
     getSelectedSegments,
+    isUnsafeUrl,
     mergeModel,
 } from 'roosterjs-content-model-dom';
 import type { ContentModelLink, IEditor } from 'roosterjs-content-model-types';
@@ -41,7 +41,7 @@ export function insertLink(
 ) {
     editor.focus();
 
-    const url = (checkXss(link) || '').trim();
+    const url = (isUnsafeUrl(link) ? '' : link || '').trim();
     if (url) {
         const linkData = matchLink(url);
         const linkUrl = linkData ? linkData.normalizedUrl : applyLinkPrefix(url);

packages/roosterjs-content-model-core/lib/command/createModelFromHtml/sanitizeElement.ts

@@ -1,4 +1,4 @@
-import { isNodeOfType } from 'roosterjs-content-model-dom';
+import { isNodeOfType, isUnsafeUrl } from 'roosterjs-content-model-dom';
 import type { ValueSanitizer } from 'roosterjs-content-model-types';
 
 /**
@@ -105,7 +105,6 @@ export const AllowedTags: ReadonlyArray<string> = [
     'ul',
     'var',
     'wbr',
-    'xmp',
 ];
 
 /**
@@ -325,7 +324,7 @@ export function createSanitizedElement(
         const name = attribute.name.toLowerCase().trim();
         const value = attribute.value;
 
-        const sanitizer = attributeSanitizers?.[name];
+        const sanitizer = getOwnValue(attributeSanitizers, name);
         const newValue =
             name == 'style'
                 ? processStyles(tag, value, styleSanitizers)
@@ -339,11 +338,7 @@ export function createSanitizedElement(
                 ? value
                 : null;
 
-        if (
-            newValue !== null &&
-            newValue !== undefined &&
-            !newValue.match(/s\n*c\n*r\n*i\n*p\n*t\n*:/i) // match script: with any NewLine inside. Browser will ignore those NewLine char and still treat it as script prefix
-        ) {
+        if (newValue !== null && newValue !== undefined && !isUnsafeUrl(newValue)) {
             element.setAttribute(name, newValue);
         }
     }
@@ -369,7 +364,7 @@ function processStyles(
                 value = processCssVariable(value);
             }
 
-            const sanitizer = styleSanitizers?.[name];
+            const sanitizer = getOwnValue(styleSanitizers, name);
             const sanitizedValue =
                 typeof sanitizer == 'function'
                     ? sanitizer(value, tagName)
@@ -383,7 +378,7 @@ function processStyles(
                 sanitizedValue != 'initial' &&
                 sanitizedValue.indexOf('expression') < 0 &&
                 !name.startsWith('-') &&
-                DefaultStyleValue[name] != sanitizedValue
+                getOwnValue(DefaultStyleValue, name) != sanitizedValue
             ) {
                 result.push(`${name}:${sanitizedValue}`);
             }
@@ -398,6 +393,13 @@ function processCssVariable(value: string): string {
     return match?.[2] || ''; // Without fallback value, we don't know what does the original value mean, so ignore it
 }
 
+// Attribute and style names come from the pasted content, so a name such as "valueOf" or
+// "hasOwnProperty" would otherwise resolve to the inherited member of Object.prototype and be
+// invoked as if it were a sanitizer, which throws and fails the whole conversion.
+function getOwnValue<T>(map: Readonly<Record<string, T>> | undefined, name: string): T | undefined {
+    return map && Object.prototype.hasOwnProperty.call(map, name) ? map[name] : undefined;
+}
+
 function isCssVariable(value: string): boolean {
     return value.indexOf(VARIABLE_PREFIX) == 0;
 }

packages/roosterjs-content-model-core/test/command/createModelFromHtml/sanitizeElementTest.ts

@@ -223,6 +223,44 @@ describe('sanitizeHtml', () => {
         runTest('aa<form action=/>cc</form>bb', 'aa<span>cc</span>bb');
     });
 
+    it('Html contains script url obfuscated with TAB or CR', () => {
+        // Browsers remove TAB, LF and CR from a URL before resolving its protocol
+        runTest('aa<a href="java&#9;script:alert(1)">cc</a>bb', 'aa<a>cc</a>bb');
+        runTest('aa<a href="javascr&#13;ipt:alert(1)">cc</a>bb', 'aa<a>cc</a>bb');
+        runTest('aa<a href="javascr&#9;ipt:alert(1)">cc</a>bb', 'aa<a>cc</a>bb');
+        runTest('aa<a href="&#32;&#9;javascript:alert(1)">cc</a>bb', 'aa<a>cc</a>bb');
+        runTest('aa<area href="java&#9;script:alert(1)">bb', 'aa<area>bb');
+    });
+
+    it('Html contains url that only looks like a script url', () => {
+        runTest(
+            '<a href="https://example.com/transcript:a">cc</a>',
+            '<a href="https://example.com/transcript:a">cc</a>'
+        );
+    });
+
+    it('Xmp is not kept because its content is serialized without escaping', () => {
+        runTest(
+            '<xmp><img src=x onerror=alert(1)></xmp>',
+            '<span>&lt;img src=x onerror=alert(1)&gt;</span>'
+        );
+    });
+
+    it('Style name that matches a member of Object.prototype', () => {
+        // Those names must not be treated as a sanitizer from the prototype chain
+        runTest('<span style="valueOf:1">aa</span>', '<span style="valueOf:1">aa</span>');
+        runTest(
+            '<span style="hasOwnProperty:1;color:red">aa</span>',
+            '<span style="hasOwnProperty:1;color:red">aa</span>'
+        );
+        runTest('<span style="toString:1">aa</span>', '<span style="toString:1">aa</span>');
+    });
+
+    it('Attribute name that matches a member of Object.prototype', () => {
+        runTest('<span constructor="1">aa</span>', '<span>aa</span>');
+        runTest('<span valueOf="1">aa</span>', '<span>aa</span>');
+    });
+
     it('Html contains unnecessary CSS', () => {
         runTest(
             '<span style="color:red">aa<span style="color:red">bb</span>cc</span>',

packages/roosterjs-content-model-core/test/command/paste/pasteTest.ts

@@ -192,7 +192,7 @@ describe('paste with content model & paste plugin', () => {
         paste(editor!, clipboardData);
 
         expect(setProcessorF.setProcessor).toHaveBeenCalledTimes(2);
-        expect(addParserF.addParser).toHaveBeenCalledTimes(DEFAULT_TIMES_ADD_PARSER_CALLED + 7);
+        expect(addParserF.addParser).toHaveBeenCalledTimes(DEFAULT_TIMES_ADD_PARSER_CALLED + 8);
         expect(WordDesktopFile.processPastedContentFromWordDesktop).toHaveBeenCalledTimes(1);
     });
 
@@ -248,7 +248,7 @@ describe('paste with content model & paste plugin', () => {
         paste(editor!, clipboardData, 'asPlainText');
 
         expect(setProcessorF.setProcessor).toHaveBeenCalledTimes(2);
-        expect(addParserF.addParser).toHaveBeenCalledTimes(12);
+        expect(addParserF.addParser).toHaveBeenCalledTimes(13);
         expect(WordDesktopFile.processPastedContentFromWordDesktop).toHaveBeenCalledTimes(1);
     });
 

packages/roosterjs-content-model-dom/lib/domUtils/isUnsafeUrl.ts

@@ -0,0 +1,33 @@
+const UnsafeProtocols = ['javascript', 'vbscript', 'livescript', 'jscript', 'mocha', 'script'];
+const IgnoredCharacters = /[\t\n\r]/g;
+const MaxControlCharCode = 0x20;
+
+/**
+ * Check whether the given URL uses a protocol that can execute script, such as "javascript:"
+ * @param url The URL to check
+ * @returns True if the URL can execute script when navigated to, otherwise false
+ * @remarks Browsers remove every TAB, LF and CR character from a URL and ignore any leading control
+ * character or space before they resolve its protocol. Those characters must be removed before the
+ * protocol is checked here, otherwise a value such as "java	script:" or "javascr
ipt:" would
+ * be treated as safe but still executed by the browser.
+ */
+export function isUnsafeUrl(url: string | null | undefined): boolean {
+    if (!url) {
+        return false;
+    }
+
+    const normalizedUrl = trimControlCharacters(url.replace(IgnoredCharacters, '')).toLowerCase();
+    const colonIndex = normalizedUrl.indexOf(':');
+
+    return colonIndex >= 0 && UnsafeProtocols.indexOf(normalizedUrl.substring(0, colonIndex)) >= 0;
+}
+
+function trimControlCharacters(url: string): string {
+    let start = 0;
+
+    while (start < url.length && url.charCodeAt(start) <= MaxControlCharCode) {
+        start++;
+    }
+
+    return url.substring(start);
+}

packages/roosterjs-content-model-dom/lib/formatHandlers/segment/linkFormatHandler.ts

@@ -1,4 +1,5 @@
 import { isElementOfType } from '../../domUtils/isElementOfType';
+import { isUnsafeUrl } from '../../domUtils/isUnsafeUrl';
 import type { FormatHandler } from '../FormatHandler';
 import type { LinkFormat } from 'roosterjs-content-model-types';
 
@@ -20,7 +21,7 @@ export const linkFormatHandler: FormatHandler<LinkFormat> = {
                 format.name = name;
             }
 
-            if (href) {
+            if (href && !isUnsafeUrl(href)) {
                 format.href = href;
             }
 
@@ -47,7 +48,7 @@ export const linkFormatHandler: FormatHandler<LinkFormat> = {
     },
     apply: (format, element) => {
         if (isElementOfType(element, 'a') && (format.href || format.name)) {
-            if (format.href) {
+            if (format.href && !isUnsafeUrl(format.href)) {
                 element.href = format.href;
             }
 

packages/roosterjs-content-model-dom/lib/index.ts

@@ -19,6 +19,7 @@ export { buildSelectionMarker } from './domToModel/utils/buildSelectionMarker';
 export { updateMetadata, getMetadata, hasMetadata } from './modelApi/metadata/updateMetadata';
 export { isNodeOfType } from './domUtils/isNodeOfType';
 export { isElementOfType } from './domUtils/isElementOfType';
+export { isUnsafeUrl } from './domUtils/isUnsafeUrl';
 export { getObjectKeys } from './domUtils/getObjectKeys';
 export { getSafeIdSelector } from './domUtils/getSafeIdSelector';
 export { toArray } from './domUtils/toArray';

packages/roosterjs-content-model-dom/test/domUtils/isUnsafeUrlTest.ts

@@ -0,0 +1,49 @@
+import { isUnsafeUrl } from '../../lib/domUtils/isUnsafeUrl';
+
+describe('isUnsafeUrl', () => {
+    it('Empty values', () => {
+        expect(isUnsafeUrl('')).toBeFalse();
+        expect(isUnsafeUrl(null)).toBeFalse();
+        expect(isUnsafeUrl(undefined)).toBeFalse();
+    });
+
+    it('Safe urls', () => {
+        expect(isUnsafeUrl('https://example.com')).toBeFalse();
+        expect(isUnsafeUrl('http://example.com')).toBeFalse();
+        expect(isUnsafeUrl('mailto:[email protected]')).toBeFalse();
+        expect(isUnsafeUrl('data:image/png;base64,AAAA')).toBeFalse();
+        expect(isUnsafeUrl('/relative/path')).toBeFalse();
+        expect(isUnsafeUrl('relative/path')).toBeFalse();
+    });
+
+    it('Safe urls that look similar to a script protocol', () => {
+        expect(isUnsafeUrl('scripting:https://example.com')).toBeFalse();
+        expect(isUnsafeUrl('transcript:https://example.com')).toBeFalse();
+        expect(isUnsafeUrl('https://example.com/script:.js')).toBeFalse();
+        expect(isUnsafeUrl('/redirect?url=javascript:alert(1)')).toBeFalse();
+    });
+
+    it('Unsafe protocols', () => {
+        expect(isUnsafeUrl('javascript:alert(1)')).toBeTrue();
+        expect(isUnsafeUrl('vbscript:msgbox(1)')).toBeTrue();
+        expect(isUnsafeUrl('script:alert(1)')).toBeTrue();
+        expect(isUnsafeUrl('JaVaScRiPt:alert(1)')).toBeTrue();
+    });
+
+    it('Unsafe protocols obfuscated with characters that browsers ignore', () => {
+        // Browsers strip TAB, LF and CR from anywhere inside a URL
+        expect(isUnsafeUrl('java\tscript:alert(1)')).toBeTrue();
+        expect(isUnsafeUrl('javascr\ript:alert(1)')).toBeTrue();
+        expect(isUnsafeUrl('javascr\nipt:alert(1)')).toBeTrue();
+        expect(isUnsafeUrl('j\ta\nv\ra\ts\nc\rr\ti\np\rt:alert(1)')).toBeTrue();
+
+        // Cases previously covered by checkXss
+        expect(isUnsafeUrl('s\nc\nr\ni\np\nt:https://example.com')).toBeTrue();
+        expect(isUnsafeUrl('S\nC\nr\ni\nP\nt:https://example.com')).toBeTrue();
+
+        // Browsers ignore leading control characters and spaces
+        expect(isUnsafeUrl('   javascript:alert(1)')).toBeTrue();
+        expect(isUnsafeUrl('\u0001javascript:alert(1)')).toBeTrue();
+        expect(isUnsafeUrl(' java\tscript:alert(1)')).toBeTrue();
+    });
+});

packages/roosterjs-content-model-plugins/lib/edit/tabUtils/handleTabOnParagraph.ts

@@ -102,6 +102,7 @@ export function handleTabOnParagraph(
                 if (markerIndex <= 0) {
                     return false;
                 }
+
                 const tabText = paragraph.segments[markerIndex - 1];
                 const tabSpacesLength = tabSpaces.length;
 
@@ -117,6 +118,8 @@ export function handleTabOnParagraph(
                     } else {
                         return false;
                     }
+                } else {
+                    return false;
                 }
             }
         }

packages/roosterjs-content-model-plugins/lib/paste/WordDesktop/processPastedContentFromWordDesktop.ts

@@ -11,6 +11,7 @@ import { setProcessor } from '../utils/setProcessor';
 import { wordContainerParser } from '../parsers/wordContainerParser';
 import { wordTableParser } from '../parsers/wordTableParser';
 import { removeListParagraphMargins } from './removeListParagraphMargins';
+import { wordBorderParser } from './wordBorderParser';
 import type { WordMetadata } from './WordMetadata';
 import type { CssRule, DomToModelOption, ElementProcessor } from 'roosterjs-content-model-types';
 
@@ -38,6 +39,7 @@ export function processPastedContentFromWordDesktop(
     addParser(domToModelOption, 'listLevel', listLevelParser);
     addParser(domToModelOption, 'container', wordContainerParser);
     addParser(domToModelOption, 'table', wordTableParser);
+    addParser(domToModelOption, 'tableBorder', wordBorderParser);
 }
 
 const wordDesktopElementProcessor = (