diff --git a/.github/workflows/e2e-db-matrix.yml b/.github/workflows/e2e-db-matrix.yml index 2f6ff001a7..ae68025136 100644 --- a/.github/workflows/e2e-db-matrix.yml +++ b/.github/workflows/e2e-db-matrix.yml @@ -23,6 +23,9 @@ on: - doc/deploy/docker-compose.*.yml workflow_dispatch: +permissions: + contents: read + concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true diff --git a/public/app/adapters/LinkValidationAdapter.js b/public/app/adapters/LinkValidationAdapter.js index 156b3ce854..b438fb480e 100644 --- a/public/app/adapters/LinkValidationAdapter.js +++ b/public/app/adapters/LinkValidationAdapter.js @@ -141,7 +141,8 @@ export default class LinkValidationAdapter { /** * Remove invalid/non-validatable links - * Filters out: empty, anchors (#), javascript:, data: URLs + * Filters out: empty, anchors (#), and dangerous script schemes + * (javascript:, vbscript:, data:). * @param {Array<{url: string, count: number}>} links * @returns {Array<{url: string, count: number}>} * @private @@ -150,12 +151,53 @@ export default class LinkValidationAdapter { return links.filter((link) => { if (!link.url || link.url.trim() === '') return false; if (link.url.startsWith('#')) return false; - if (link.url.startsWith('javascript:')) return false; - if (link.url.startsWith('data:')) return false; + if (this._hasDangerousScheme(link.url)) return false; return true; }); } + /** + * Detect whether a URL uses a dangerous script-bearing scheme + * (javascript:, vbscript:, data:). + * + * Normalizes the URL before matching so the check cannot be bypassed + * with mixed case, leading/embedded whitespace and control characters, + * or HTML character entities (e.g. "javascript:"). + * + * @param {string} url + * @returns {boolean} true when the URL resolves to a dangerous scheme + * @private + */ + _hasDangerousScheme(url) { + if (typeof url !== 'string') return false; + + // Safely convert a numeric code point, returning '' for invalid or + // out-of-range values so String.fromCodePoint cannot throw a + // RangeError (e.g. on "�" or "�"). Dropping such + // junk only makes scheme detection stricter, never weaker. + const safeFromCodePoint = (cp) => + Number.isInteger(cp) && cp >= 0 && cp <= 0x10ffff ? String.fromCodePoint(cp) : ''; + + // Decode common HTML entities (named and numeric) so encoded + // schemes such as "javascript:" are caught. + let normalized = url.replace(/&#x([0-9a-f]+);?/gi, (_m, hex) => + safeFromCodePoint(Number.parseInt(hex, 16)), + ); + normalized = normalized.replace(/&#(\d+);?/g, (_m, dec) => + safeFromCodePoint(Number.parseInt(dec, 10)), + ); + const namedEntities = { amp: '&', lt: '<', gt: '>', quot: '"', apos: "'", colon: ':', tab: '\t', newline: '\n' }; + normalized = normalized.replace(/&(amp|lt|gt|quot|apos|colon|tab|newline);/gi, (_m, name) => namedEntities[name.toLowerCase()] ?? _m); + + // Strip whitespace and control characters (incl. NUL and DEL) that + // browsers ignore when resolving the scheme, then lowercase for + // comparison. \x00-\x20 covers C0 controls + space; \x7f is DEL. + // eslint-disable-next-line no-control-regex + const stripped = normalized.replace(/[\x00-\x20\x7f]+/g, '').toLowerCase(); + + return stripped.startsWith('javascript:') || stripped.startsWith('vbscript:') || stripped.startsWith('data:'); + } + /** * Deduplicate links, keeping the one with highest count * @param {Array<{url: string, count: number}>} links diff --git a/public/app/adapters/LinkValidationAdapter.test.js b/public/app/adapters/LinkValidationAdapter.test.js index 611c58b1a3..d3dd88418e 100644 --- a/public/app/adapters/LinkValidationAdapter.test.js +++ b/public/app/adapters/LinkValidationAdapter.test.js @@ -485,6 +485,73 @@ describe('LinkValidationAdapter', () => { expect(result.length).toBe(1); expect(result[0].url).toBe('https://valid.com'); }); + + it('should reject vbscript:, mixed-case, whitespace-prefixed and entity-encoded script schemes', () => { + const dangerous = [ + { url: 'vbscript:msgbox(1)', count: 1 }, + { url: 'VBScript:MsgBox(1)', count: 1 }, + { url: 'JavaScript:alert(1)', count: 1 }, + { url: ' javascript:alert(1)', count: 1 }, + { url: '\t\njavascript:alert(1)', count: 1 }, + { url: 'java\x00script:alert(1)', count: 1 }, + { url: 'DATA:text/html,', count: 1 }, + { url: 'javascript:alert(1)', count: 1 }, + { url: 'javascript:alert(1)', count: 1 }, + ]; + const result = adapter._removeInvalidLinks(dangerous); + + expect(result).toEqual([]); + }); + + it('should preserve legitimate safe links', () => { + const safe = [ + { url: 'http://example.com', count: 1 }, + { url: 'https://example.com/page', count: 1 }, + { url: '//cdn.example.com/file.js', count: 1 }, + { url: 'images/photo.jpg', count: 1 }, + { url: 'mailto:user@example.com', count: 1 }, + { url: 'tel:+1234567890', count: 1 }, + ]; + const result = adapter._removeInvalidLinks(safe); + + expect(result.map((l) => l.url)).toEqual([ + 'http://example.com', + 'https://example.com/page', + '//cdn.example.com/file.js', + 'images/photo.jpg', + 'mailto:user@example.com', + 'tel:+1234567890', + ]); + }); + }); + + describe('_hasDangerousScheme', () => { + it('should detect dangerous schemes through obfuscation', () => { + expect(adapter._hasDangerousScheme('javascript:alert(1)')).toBe(true); + expect(adapter._hasDangerousScheme('vbscript:msgbox(1)')).toBe(true); + expect(adapter._hasDangerousScheme('data:text/html,abc')).toBe(true); + expect(adapter._hasDangerousScheme(' JAVASCRIPT:alert(1)')).toBe(true); + expect(adapter._hasDangerousScheme('javascript:alert(1)')).toBe(true); + expect(adapter._hasDangerousScheme('javascript:alert(1)')).toBe(true); + }); + + it('should not flag safe links and non-string input', () => { + expect(adapter._hasDangerousScheme('https://example.com')).toBe(false); + expect(adapter._hasDangerousScheme('images/photo.jpg')).toBe(false); + expect(adapter._hasDangerousScheme('mailto:user@example.com')).toBe(false); + expect(adapter._hasDangerousScheme(null)).toBe(false); + }); + + it('should not throw on out-of-range numeric entities', () => { + // String.fromCodePoint throws RangeError for code points > 0x10FFFF + // or negative; the guard must drop them instead of crashing. + expect(() => adapter._hasDangerousScheme('�')).not.toThrow(); + expect(() => adapter._hasDangerousScheme('�')).not.toThrow(); + expect(adapter._hasDangerousScheme('�')).toBe(false); + expect(adapter._hasDangerousScheme('�')).toBe(false); + // A real scheme padded with an out-of-range entity is still caught. + expect(adapter._hasDangerousScheme('�javascript:alert(1)')).toBe(true); + }); }); describe('_deduplicateLinks', () => { diff --git a/public/app/common/LatexPreRenderer.js b/public/app/common/LatexPreRenderer.js index d725540ba4..ff71b69093 100644 --- a/public/app/common/LatexPreRenderer.js +++ b/public/app/common/LatexPreRenderer.js @@ -123,18 +123,26 @@ let clean = latexWithHtml.replace(//gi, '\n'); // Remove any other HTML tags FIRST (before decoding entities) - // This prevents decoded < and > from being mistaken for tags - clean = clean.replace(/<[^>]+>/g, ''); - - // Decode common HTML entities AFTER removing tags + // This prevents decoded < and > from being mistaken for tags. + // Apply repeatedly until the string stops changing so that removing one + // tag cannot splice two halves into a new tag (e.g. "<b>" -> "b>"). + let prevClean; + do { + prevClean = clean; + clean = clean.replace(/<[^>]+>/g, ''); + } while (clean !== prevClean); + + // Decode common HTML entities AFTER removing tags. + // Decode & LAST so that an already-escaped sequence like "&lt;" + // (the literal text "<") does not get double-decoded into "<". clean = clean .replace(/ /gi, ' ') .replace(/</g, '<') .replace(/>/g, '>') - .replace(/&/g, '&') .replace(/"/g, '"') .replace(/&#(\d+);/g, (_, code) => String.fromCharCode(parseInt(code, 10))) - .replace(/&#x([a-fA-F0-9]+);/g, (_, code) => String.fromCharCode(parseInt(code, 16))); + .replace(/&#x([a-fA-F0-9]+);/g, (_, code) => String.fromCharCode(parseInt(code, 16))) + .replace(/&/g, '&'); return clean; } @@ -711,7 +719,7 @@ function isNumberedEquation(latex) { const envMatch = latex.match(/\\begin\{([^}*]+)\*?\}/); if (!envMatch) return false; - const envName = envMatch[1].replace('*', ''); // Remove * for unnumbered variants + const envName = envMatch[1].replace(/\*/g, ''); // Remove * for unnumbered variants return NUMBERED_EQUATION_ENVS.has(envName) && !envMatch[1].endsWith('*'); } @@ -784,6 +792,14 @@ * @returns {Promise<{html: string, hasLatex: boolean, latexRendered: boolean, count: number}>} */ async function preRenderPerIdevice(html, preserved) { + // Security note (stored-xss): DOMParser.parseFromString builds an INERT + // document - scripts are NOT executed here, the document is only used to + // locate and re-serialize content. The ONLY new HTML this routine injects + // back via innerHTML is the MathJax library-rendered / wrapper + // produced by createRenderedWrapperHtml(), whose data-latex attribute is + // escaped through escapeHtmlAttribute(). Every other byte is a faithful + // round-trip of the author's own already-HTML content (the trusted source + // of their own page), so this transformation introduces no new XSS sink. const parser = new DOMParser(); const doc = parser.parseFromString(html, 'text/html'); @@ -925,7 +941,13 @@ return await preRenderPerIdevice(safeHtml, preserved); } - // Parse HTML into DOM (simple mode without iDevice scoping) + // Parse HTML into DOM (simple mode without iDevice scoping). + // Security note (stored-xss): identical reasoning to preRenderPerIdevice - + // the parsed document is INERT (no script execution), only used to find + // and re-serialize content. The single new HTML payload injected via + // innerHTML is the MathJax library-rendered / wrapper with an + // escaped data-latex attribute; the remaining output is a faithful + // round-trip of the author's own already-HTML content. const parser = new DOMParser(); const doc = parser.parseFromString(safeHtml, 'text/html'); diff --git a/public/app/common/LatexPreRenderer.test.js b/public/app/common/LatexPreRenderer.test.js index c6bb2a8840..66a61e8519 100644 --- a/public/app/common/LatexPreRenderer.test.js +++ b/public/app/common/LatexPreRenderer.test.js @@ -252,6 +252,44 @@ describe('LatexPreRenderer', () => { expect(result).toContain('\\begin{aligned}'); expect(result).toContain('x &= 1'); }); + + // Security: incomplete-multi-character-sanitization (CodeQL). + // A single-pass tag strip can be defeated because removing one match + // can splice two halves into a NEW tag. The strip must run to a fixed + // point so no b'; + const result = LatexPreRenderer._cleanLatexFromHtml(input); + + // The security property: removing one tag must not splice halves into + // a surviving " { + const input = '<b>x = 1<src>'; + const result = LatexPreRenderer._cleanLatexFromHtml(input); + + // After fixed-point removal no opening "<" (start of a tag) may remain. + expect(result).not.toContain('<'); + expect(result).toContain('x = 1'); + }); + + // Security: double-escaping (CodeQL). Decoding & must happen LAST so + // an already-escaped sequence like "&lt;" (the literal text "<") + // is not double-decoded into "<". + test('does not double-decode &lt; into <', () => { + expect(LatexPreRenderer._cleanLatexFromHtml('&lt;')).toBe('<'); + }); + + test('does not double-decode &amp; into &', () => { + expect(LatexPreRenderer._cleanLatexFromHtml('&amp;')).toBe('&'); + }); + + test('still decodes a single & to & (legitimate input preserved)', () => { + expect(LatexPreRenderer._cleanLatexFromHtml('a && b')).toBe('a && b'); + }); }); describe('preRender', () => { @@ -421,6 +459,40 @@ describe('LatexPreRenderer', () => { expect(result.html).not.toContain('data-latex="\\[
'); }); + // Security: stored-xss (CodeQL) on the DOMParser round-trip. + // The only NEW HTML preRender injects is the MathJax library-rendered + // wrapper, with its data-latex attribute escaped via escapeHtmlAttribute(). + // LaTeX that contains a quote/angle-bracket payload must be neutralised in + // the attribute (escaped), never emitted as a live attribute/tag. + test('escapes quote/angle payloads inside the injected math wrapper attribute', async () => { + const html = '

\\(x" onmouseover="alert(1)\\)

'; + const result = await LatexPreRenderer.preRender(html); + + expect(result.latexRendered).toBe(true); + // The injected wrapper is present and library-rendered. + expect(result.html).toContain('exe-math-rendered'); + // The wrapper's own data-latex attribute (built by THIS file via + // escapeHtmlAttribute) must entity-escape the double quote so the + // payload cannot break out into a live onmouseover handler. + const dataLatexMatch = result.html.match(/data-latex="([^"]*)"/); + expect(dataLatexMatch).not.toBeNull(); + expect(dataLatexMatch[1]).toContain('" onmouseover="alert(1)'); + expect(dataLatexMatch[1]).not.toContain('"'); + }); + + test('does not introduce a new live (note: space inside tag is common) html = html.replace( - /]*src=["']([^"']+)["'][^>]*>[^<]*<\/script>/gi, + /]*src=["']([^"']+)["'][^>]*>[^<]*<\/script\s*>/gi, (match, src) => { const content = this._decodeFileContent(this._findFileContent(files, src)); if (content) { diff --git a/public/app/workarea/interface/elements/previewPanel.test.js b/public/app/workarea/interface/elements/previewPanel.test.js index 386cca8488..357e0e2f2a 100644 --- a/public/app/workarea/interface/elements/previewPanel.test.js +++ b/public/app/workarea/interface/elements/previewPanel.test.js @@ -1595,6 +1595,28 @@ describe('PreviewPanelManager', () => { expect(result).not.toContain('src="app.js"'); }); + it('should inline JS script tags with whitespace before the closing angle bracket', () => { + const html = ''; + const files = { 'app.js': 'console.log("hello")' }; + + const result = manager._inlineResources(html, files); + + expect(result).toContain(' tag from an HTML string. + * + * The end-tag pattern tolerates optional whitespace before the closing angle + * bracket (e.g. ``) and is case-insensitive, so real-world end tags + * are not missed (bad-tag-filter). The replacement is applied repeatedly until + * the string stops changing, so a nested/obfuscated payload such as + * `ipt>` cannot splice two halves into a fresh `

bye

'); + expect(out).toBe('

hi

bye

'); + expect(out.toLowerCase()).not.toContain(' { + const out = stripScriptTags('after'); + expect(out).toBe('after'); + expect(out.toLowerCase()).not.toContain(' { + const out = stripScriptTags('beforedone'); + expect(out.toLowerCase()).not.toContain(' { + const html = '

keep me

'; + expect(stripScriptTags(html)).toBe(html); + }); + + it('does not strip text that merely mentions script without a real tag', () => { + const html = '

The word script is fine here

'; + expect(stripScriptTags(html)).toBe(html); + }); +}); diff --git a/public/files/perm/idevices/base/adaptative-quiz/edition/adaptative-quiz.js b/public/files/perm/idevices/base/adaptative-quiz/edition/adaptative-quiz.js index 90124ad4ae..4855ba86cd 100644 --- a/public/files/perm/idevices/base/adaptative-quiz/edition/adaptative-quiz.js +++ b/public/files/perm/idevices/base/adaptative-quiz/edition/adaptative-quiz.js @@ -158,10 +158,15 @@ var $exeDevice = { normalizeQuestionsPerRound: value => Math.max(parseInt(value, 10) || 1, 1), - removeTags: str => - String(str ?? '') - .replace(/<[^>]*>/g, '') - .trim(), + removeTags: str => { + let result = String(str ?? ''); + let prev; + do { + prev = result; + result = result.replace(/<[^>]*>/g, ''); + } while (result !== prev); + return result.trim(); + }, /** * Reusable audio input with voice recorder, matching quick-questions' diff --git a/public/files/perm/idevices/base/adaptative-quiz/edition/adaptative-quiz.test.js b/public/files/perm/idevices/base/adaptative-quiz/edition/adaptative-quiz.test.js index 5ee6a7b9ab..b63341f97d 100644 --- a/public/files/perm/idevices/base/adaptative-quiz/edition/adaptative-quiz.test.js +++ b/public/files/perm/idevices/base/adaptative-quiz/edition/adaptative-quiz.test.js @@ -1936,4 +1936,33 @@ describe('adaptative-quiz edition', () => { expect(validateCalls).toBe(0); }); }); + + describe('removeTags', () => { + it('strips a simple tag and trims the remaining text', () => { + expect(idevice.removeTags('

Hello

')).toBe('Hello'); + }); + + it('returns an empty string for nullish input', () => { + expect(idevice.removeTags(null)).toBe(''); + expect(idevice.removeTags(undefined)).toBe(''); + }); + + it('leaves no live ', + '</script>', + 'ipt>alert(1)', + ]; + for (const payload of payloads) { + expect(idevice.removeTags(payload).toLowerCase()).not.toContain(' { + expect(idevice.removeTags(' Plain question text ')).toBe('Plain question text'); + expect(idevice.removeTags('2 plus 3 equals 5')).toBe('2 plus 3 equals 5'); + }); + }); }); diff --git a/public/files/perm/idevices/base/az-quiz-game/edition/az-quiz-game.js b/public/files/perm/idevices/base/az-quiz-game/edition/az-quiz-game.js index e926b0e4ed..be5feeaacf 100644 --- a/public/files/perm/idevices/base/az-quiz-game/edition/az-quiz-game.js +++ b/public/files/perm/idevices/base/az-quiz-game/edition/az-quiz-game.js @@ -1212,11 +1212,16 @@ var $exeDevice = { const words = []; $entries.find('ENTRY').each(function () { - const concept = $(this).find('CONCEPT').text(), - definition = $(this) - .find('DEFINITION') - .text() - .replace(/<[^>]*>/g, ''); + const concept = $(this).find('CONCEPT').text(); + let definition = $(this).find('DEFINITION').text(); + // Strip HTML tags repeatedly: removing one tag can splice + // surrounding characters into a new tag (e.g. "<script>"), + // so loop until the string stops changing. + let prevDefinition; + do { + prevDefinition = definition; + definition = definition.replace(/<[^>]*>/g, ''); + } while (definition !== prevDefinition); if (concept && definition) { words.push(`${concept}#${definition}`); } diff --git a/public/files/perm/idevices/base/az-quiz-game/edition/az-quiz-game.test.js b/public/files/perm/idevices/base/az-quiz-game/edition/az-quiz-game.test.js index f0294d99ba..f1549c393e 100644 --- a/public/files/perm/idevices/base/az-quiz-game/edition/az-quiz-game.test.js +++ b/public/files/perm/idevices/base/az-quiz-game/edition/az-quiz-game.test.js @@ -32,7 +32,13 @@ function loadIdevice(code) { // Helper to strip HTML tags const stripTags = (html) => { if (!html) return ''; - return String(html).replace(/<[^>]*>/g, ''); + let result = String(html); + let prev; + do { + prev = result; + result = result.replace(/<[^>]*>/g, ''); + } while (result !== prev); + return result; }; // Mock jQuery with chaining support @@ -525,4 +531,57 @@ describe('az-quiz-game iDevice', () => { expect(typeof $exeDevice.validateData).toBe('function'); }); }); + + describe('mock $ text() stripTags helper', () => { + it('strips a simple HTML tag from selector text', () => { + expect(global.$('

Hello

').text()).toBe('Hello'); + }); + + it('strips nested/obfuscated tags, leaving no complete tag behind', () => { + // The fixed-point loop keeps stripping until the string stops + // changing, so a nested/obfuscated payload cannot leave a complete + // `<...>` tag (in particular, no `'; + const result = global.$(payload).text(); + // No surviving complete HTML tag, and no `'); + expect(result).not.toMatch(/<[^>]*>/); + }); + + it('leaves plain text without tags unchanged', () => { + expect(global.$('Plain text').text()).toBe('Plain text'); + }); + }); + + describe('importGlosary definition sanitization', () => { + // importGlosary strips HTML tags from each DEFINITION using a + // fixed-point loop so that removing one tag cannot splice surrounding + // characters into a new tag. This mirrors the source logic and asserts + // the security property on an obfuscated payload while preserving + // legitimate text. + const stripDefinition = (text) => { + let definition = text; + let prevDefinition; + do { + prevDefinition = definition; + definition = definition.replace(/<[^>]*>/g, ''); + } while (definition !== prevDefinition); + return definition; + }; + + it('leaves no complete tag behind for an obfuscated payload', () => { + const payload = 'ipt>The Sun'; + const result = stripDefinition(payload); + expect(result.toLowerCase()).not.toContain(''; + const cleaned = $exeDevice.stripTags(payload); + // The dangerous ") is left behind. + expect(cleaned).not.toMatch(/<[^>]*>/); + }); + + it('reaches a fixed point: re-stripping the output is a no-op', () => { + const payload = '<
script'; + const once = $exeDevice.stripTags(payload); + // A single-pass strip can splice new tags; the loop must converge. + expect(once).not.toMatch(/<[^>]*>/); + expect(once.replace(/<[^>]*>/g, '')).toBe(once); + }); + }); + describe('validTime', () => { it('returns true for valid time format hh:mm:ss', () => { expect($exeDevice.validTime('00:00:00')).toBe(true); diff --git a/public/files/perm/idevices/base/discover/edition/discover.js b/public/files/perm/idevices/base/discover/edition/discover.js index 6b9fc173fa..fc6fcc7caf 100644 --- a/public/files/perm/idevices/base/discover/edition/discover.js +++ b/public/files/perm/idevices/base/discover/edition/discover.js @@ -1723,11 +1723,15 @@ var $exeDevice = { const cardsJson = $entries .find('ENTRY') .map((_, entry) => { - const concept = $(entry).find('CONCEPT').text(), - definition = $(entry) - .find('DEFINITION') - .text() - .replace(/<[^>]*>/g, ''); + const concept = $(entry).find('CONCEPT').text(); + let definition = $(entry).find('DEFINITION').text(), + prevDefinition; + // Re-apply the tag strip until the result stabilises so that + // overlapping/nested markup cannot splice into a new tag. + do { + prevDefinition = definition; + definition = definition.replace(/<[^>]*>/g, ''); + } while (definition !== prevDefinition); return concept && definition ? { eText: concept, eTextBk: definition } : null; diff --git a/public/files/perm/idevices/base/discover/edition/discover.test.js b/public/files/perm/idevices/base/discover/edition/discover.test.js index f790642bf6..28d5de4b35 100644 --- a/public/files/perm/idevices/base/discover/edition/discover.test.js +++ b/public/files/perm/idevices/base/discover/edition/discover.test.js @@ -82,9 +82,15 @@ const $exeDevice = { removeTags: function (str) { const wrapper = { html: function(content) { this.content = content; }, - text: function() { + text: function() { if (!this.content) return ''; - return this.content.replace(/<[^>]*>/g, ''); + let result = this.content; + let prev; + do { + prev = result; + result = result.replace(/<[^>]*>/g, ''); + } while (result !== prev); + return result; } }; wrapper.html(str); @@ -294,6 +300,21 @@ describe('Discover Edition Functions', () => { expect($exeDevice.removeTags('
Content
')).toBe('Content'); expect($exeDevice.removeTags('Link')).toBe('Link'); }); + + it('should leave no '); + expect(result.toLowerCase()).not.toContain(' { + // Re-sanitizing the output must not change it further, and no + // residual tag-opening "<" may survive a spliced payload. + const once = $exeDevice.removeTags('>ce'); + const twice = $exeDevice.removeTags(once); + expect(twice).toBe(once); + expect(once).not.toContain('<'); + }); }); describe('escapeQuotes', () => { @@ -494,6 +515,67 @@ describe('Discover Edition Functions', () => { }); }); + describe('importGlosary sanitization', () => { + // importGlosary parses real XML and uses jQuery DOM traversal, so it + // needs the genuine jQuery/DOMParser. The file-level mock above replaces + // global.$ (and, since window === globalThis here, window.$) with a + // vi.fn stub, but the real jQuery is still reachable via global.jQuery + // and the real DOMParser via global.DOMParser (both set by the setup). + const xmlEscape = (s) => + s + .replace(/&/g, '&') + .replace(//g, '>'); + // The payload is XML-escaped so that .text() yields it back as literal + // characters that reach the regex strip (otherwise the XML parser would + // treat it as markup and discard the tags itself). + const buildGlosaryXml = (definitionInner) => + `Term` + + `${xmlEscape(definitionInner)}`; + + const importWithRealJQuery = (xml) => { + const savedDollar = global.$; + global.$ = global.jQuery; + try { + const realExeDevice = global.loadIdevice( + join(__dirname, 'discover.js') + ); + let captured = null; + realExeDevice.insertCards = (cards) => { + captured = cards; + }; + realExeDevice.importGlosary(xml); + return captured; + } finally { + global.$ = savedDollar; + } + }; + + it('strips legitimate inline markup from definitions', () => { + const cards = importWithRealJQuery( + buildGlosaryXml('A bold definition') + ); + expect(cards).toEqual([ + { eText: 'Term', eTextBk: 'A bold definition' }, + ]); + }); + + it('fully removes nested/obfuscated tag payloads (no safe') + ); + expect(cards).toHaveLength(1); + const definition = cards[0].eTextBk; + expect(definition.toLowerCase()).not.toContain(' { it('downloads question text with the discover filename and container', () => { const realExeDevice = global.loadIdevice(join(__dirname, 'discover.js')); diff --git a/public/files/perm/idevices/base/dragdrop/edition/dragdrop.js b/public/files/perm/idevices/base/dragdrop/edition/dragdrop.js index c125100b81..8817b5071d 100644 --- a/public/files/perm/idevices/base/dragdrop/edition/dragdrop.js +++ b/public/files/perm/idevices/base/dragdrop/edition/dragdrop.js @@ -460,11 +460,11 @@ var $exeDevice = { }, decodeURIComponentSafe: function (s) { - return s ? decodeURIComponent(s).replace('%', '%') : s; + return s ? decodeURIComponent(s).replace(/%/g, '%') : s; }, encodeURIComponentSafe: function (s) { - return s ? encodeURIComponent(s.replace('%', '%')) : s; + return s ? encodeURIComponent(s.replace(/%/g, '%')) : s; }, validateCard: function () { diff --git a/public/files/perm/idevices/base/dragdrop/edition/dragdrop.test.js b/public/files/perm/idevices/base/dragdrop/edition/dragdrop.test.js index 71a3657e8d..de5db0c2a9 100644 --- a/public/files/perm/idevices/base/dragdrop/edition/dragdrop.test.js +++ b/public/files/perm/idevices/base/dragdrop/edition/dragdrop.test.js @@ -41,6 +41,35 @@ describe('dragdrop iDevice export helpers', () => { expect(downloadBlob.mock.calls[0][1]).toBe('Activity-DragDrop.json'); expect(downloadBlob.mock.calls[0][2]).toBe('dragdropQIdeviceForm'); }); + + it('escapes every percent sign when encoding, not just the first', () => { + // Multiple '%' must all be escaped (global flag), otherwise the + // unescaped ones become decoder control characters. + const encoded = $exeDevice.encodeURIComponentSafe('100% off 50% sale'); + expect(encoded).not.toContain('%25'); + expect(encoded).toBe(encodeURIComponent('100% off 50% sale')); + }); + + it('round-trips strings containing multiple percent signs', () => { + const original = 'a%b%c%d'; + const restored = $exeDevice.decodeURIComponentSafe( + $exeDevice.encodeURIComponentSafe(original) + ); + expect(restored).toBe(original); + }); + + it('preserves legitimate input without percent signs', () => { + const original = 'https://example.com/image (1).png'; + const restored = $exeDevice.decodeURIComponentSafe( + $exeDevice.encodeURIComponentSafe(original) + ); + expect(restored).toBe(original); + }); + + it('returns falsy input unchanged for both safe helpers', () => { + expect($exeDevice.encodeURIComponentSafe('')).toBe(''); + expect($exeDevice.decodeURIComponentSafe('')).toBe(''); + }); }); // Handlers whose callback is a single guarded call: [selector, event, method] diff --git a/public/files/perm/idevices/base/electrical-circuits/edition/electrical-circuits.js b/public/files/perm/idevices/base/electrical-circuits/edition/electrical-circuits.js index 9f4a7ac9c1..08df988777 100644 --- a/public/files/perm/idevices/base/electrical-circuits/edition/electrical-circuits.js +++ b/public/files/perm/idevices/base/electrical-circuits/edition/electrical-circuits.js @@ -2688,11 +2688,17 @@ var $exeDevice = { const questionsJson = []; $entries.find('ENTRY').each(function () { const $this = $(this), - concept = $this.find('CONCEPT').text(), - definition = $this - .find('DEFINITION') - .text() - .replace(/<[^>]*>/g, ''); + concept = $this.find('CONCEPT').text(); + // Strip HTML tags. Apply the replacement repeatedly until the + // string stops changing, because removing one match can splice the + // remaining characters into a new tag (e.g. "<script>" -> + // "safe". The strip + // (looped to a fixed point) must remove every "<...>" tag so no + // " { + const xml = buildGlosaryXml('Term', 'A plain definition'); + $exeDevice.importGlosary(xml); + expect(captured.length).toBe(1); + expect(captured[0].eText).toBe('Term'); + expect(captured[0].eTextBk).toBe('A plain definition'); + }); + + it('returns false for malformed XML', () => { + expect($exeDevice.importGlosary('not xml at all <<')).toBe(false); + }); + }); + describe('addEvents', () => { let originalExeDevicesEdition; diff --git a/public/files/perm/idevices/base/guess/edition/guess.js b/public/files/perm/idevices/base/guess/edition/guess.js index d8741e6308..b78476f2c4 100644 --- a/public/files/perm/idevices/base/guess/edition/guess.js +++ b/public/files/perm/idevices/base/guess/edition/guess.js @@ -2552,11 +2552,15 @@ var $exeDevice = { const words = []; $entries.find('ENTRY').each(function () { - const concept = $(this).find('CONCEPT').text(), - definition = $(this) - .find('DEFINITION') - .text() - .replace(/<[^>]*>/g, ''); + const concept = $(this).find('CONCEPT').text(); + // Strip HTML tags repeatedly until stable: a single pass can splice + // two remaining halves into a new tag (e.g. <script> -> safe + const xml = buildGlosaryXml([ + { + concept: 'attack', + definition: + '<scr<script>ipt>alert(1)</script>safe', + }, + ]); + $exeDevice.importGlosary(xml); + expect(captured).toHaveLength(1); + // No residual opening-tag markup must remain after the fixed-point loop. + expect(captured[0].definition.indexOf('<')).toBe(-1); + expect(/`) + // and is case-insensitive. + var prev; + var out = svg; + do { + prev = out; + out = out + .replace(//gi, '') + .replace(/<\/?\s*foreignObject[^>]*>/gi, '') + .replace(/\son[a-z]+\s*=\s*"[^"]*"/gi, '') + .replace(/\son[a-z]+\s*=\s*'[^']*'/gi, '') + // Unquoted handler values: stop at whitespace or '>' (also '/>'). + .replace(/\son[a-z]+\s*=\s*[^\s>]+/gi, '') + // Dangerous URL schemes. `javascript:` and `vbscript:` are + // always unsafe and are removed outright. For `data:` we only + // strip the script-executing `data:text/html` form so + // legitimate embedded images (`data:image/...`) keep working. + .replace(/javascript:/gi, '') + .replace(/vbscript:/gi, '') + .replace(/data:\s*text\/html/gi, ''); + } while (out !== prev); + return out; } /** diff --git a/public/files/perm/idevices/base/slide/export/slide.test.js b/public/files/perm/idevices/base/slide/export/slide.test.js index db096a03ba..d7f0ead652 100644 --- a/public/files/perm/idevices/base/slide/export/slide.test.js +++ b/public/files/perm/idevices/base/slide/export/slide.test.js @@ -120,6 +120,44 @@ describe('$slide._scrubSvg', () => { expect(_scrubSvg(42)).toBe(''); expect(_scrubSvg('')).toBe(''); }); + + it('defeats nested/spliced ipt>xyipt>'); + expect(out.toLowerCase()).not.toContain(' end tags with whitespace before the angle bracket', () => { + const out = _scrubSvg('\n'); + expect(out.toLowerCase()).not.toContain(' { + expect(_scrubSvg('')).not.toContain('vbscript:'); + expect(_scrubSvg('').toLowerCase()).not.toContain('vbscript:'); + }); + + it('defeats spliced javascript: schemes (loops to a fixed point)', () => { + const out = _scrubSvg(''); + expect(out.toLowerCase()).not.toContain('javascript:'); + }); + + it('strips script-executing data:text/html URLs', () => { + const out = _scrubSvg(''); + expect(out.toLowerCase()).not.toContain('data:text/html'); + }); + + it('keeps legitimate embedded data:image URLs intact', () => { + const svg = ''; + expect(_scrubSvg(svg)).toContain('data:image/png;base64,AAAA'); + }); + + it('strips (which can embed arbitrary HTML), matching the src twin', () => { + const out = _scrubSvg('
hi
'); + expect(out.toLowerCase()).not.toContain('foreignobject'); + }); }); // ───────────────────────────────────────────────────────────────────────────── diff --git a/public/files/perm/idevices/base/slide/src/sanitizer.test.js b/public/files/perm/idevices/base/slide/src/sanitizer.test.js index f83db5d8d7..b80ae6b654 100644 --- a/public/files/perm/idevices/base/slide/src/sanitizer.test.js +++ b/public/files/perm/idevices/base/slide/src/sanitizer.test.js @@ -40,6 +40,42 @@ describe('scrubSvg', () => { const out = scrubSvg('
'); expect(out).not.toContain('foreignObject'); }); + + it('defeats nested/spliced would splice the outer halves + // into a fresh ipt>xyipt>'); + expect(out.toLowerCase()).not.toContain(' end tags with whitespace before the angle bracket', () => { + const out = scrubSvg('\n'); + expect(out.toLowerCase()).not.toContain(' { + expect(scrubSvg('')).not.toContain('vbscript:'); + expect(scrubSvg('').toLowerCase()).not.toContain('vbscript:'); + }); + + it('defeats spliced javascript: schemes (loops to a fixed point)', () => { + // `javasjavascript:cript:` collapses to `javascript:` after one removal. + const out = scrubSvg(''); + expect(out.toLowerCase()).not.toContain('javascript:'); + }); + + it('strips script-executing data:text/html URLs', () => { + const out = scrubSvg(''); + expect(out.toLowerCase()).not.toContain('data:text/html'); + }); + + it('keeps legitimate embedded data:image URLs intact', () => { + const svg = ''; + expect(scrubSvg(svg)).toContain('data:image/png;base64,AAAA'); + }); }); describe('sanitizeSvg', () => { diff --git a/public/files/perm/idevices/base/slide/src/sanitizer.ts b/public/files/perm/idevices/base/slide/src/sanitizer.ts index 6079102c37..87edcdae9d 100644 --- a/public/files/perm/idevices/base/slide/src/sanitizer.ts +++ b/public/files/perm/idevices/base/slide/src/sanitizer.ts @@ -25,13 +25,31 @@ type DomPurifyLike = { */ export function scrubSvg(svg: unknown): string { if (!svg || typeof svg !== 'string') return ''; - return svg - .replace(//gi, '') - .replace(/<\/?\s*foreignObject[^>]*>/gi, '') - .replace(/\son[a-z]+\s*=\s*"[^"]*"/gi, '') - .replace(/\son[a-z]+\s*=\s*'[^']*'/gi, '') - .replace(/\son[a-z]+\s*=\s*[^\s>]+/gi, '') - .replace(/javascript:/gi, ''); + // Apply every removal repeatedly until the string stops changing. + // A single pass can be defeated because deleting one match can splice + // two halves into a brand-new match (e.g. `ipt>` -> ``) and is case-insensitive. + let prev: string; + let out = svg; + do { + prev = out; + out = out + .replace(//gi, '') + .replace(/<\/?\s*foreignObject[^>]*>/gi, '') + .replace(/\son[a-z]+\s*=\s*"[^"]*"/gi, '') + .replace(/\son[a-z]+\s*=\s*'[^']*'/gi, '') + .replace(/\son[a-z]+\s*=\s*[^\s>]+/gi, '') + // Dangerous URL schemes. `javascript:` and `vbscript:` are always + // unsafe and are removed outright. For `data:` we only strip the + // script-executing `data:text/html` form so legitimate embedded + // images (`data:image/...`) keep working. + .replace(/javascript:/gi, '') + .replace(/vbscript:/gi, '') + .replace(/data:\s*text\/html/gi, ''); + } while (out !== prev); + return out; } /** diff --git a/public/files/perm/idevices/base/three-sixty-viewer/edition/three-sixty-viewer.js b/public/files/perm/idevices/base/three-sixty-viewer/edition/three-sixty-viewer.js index b3d49a6d6b..5c899cfe0f 100644 --- a/public/files/perm/idevices/base/three-sixty-viewer/edition/three-sixty-viewer.js +++ b/public/files/perm/idevices/base/three-sixty-viewer/edition/three-sixty-viewer.js @@ -1042,9 +1042,13 @@ var $exeDevice = { var parts = path.split('.'); var t = target; for (var i = 0; i < parts.length - 1; i++) { - t = t[parts[i]]; + var key = parts[i]; + if (key === '__proto__' || key === 'prototype' || key === 'constructor') return; + t = t[key]; } - t[parts[parts.length - 1]] = value; + var lastKey = parts[parts.length - 1]; + if (lastKey === '__proto__' || lastKey === 'prototype' || lastKey === 'constructor') return; + t[lastKey] = value; }, escapeHtml: str => diff --git a/public/files/perm/idevices/base/three-sixty-viewer/edition/three-sixty-viewer.test.js b/public/files/perm/idevices/base/three-sixty-viewer/edition/three-sixty-viewer.test.js index 7761148599..981f9add96 100644 --- a/public/files/perm/idevices/base/three-sixty-viewer/edition/three-sixty-viewer.test.js +++ b/public/files/perm/idevices/base/three-sixty-viewer/edition/three-sixty-viewer.test.js @@ -1063,4 +1063,32 @@ describe('three-sixty-viewer iDevice (edition)', () => { }); }); }); + + describe('setNestedPath (prototype pollution safety)', () => { + it('assigns a nested value for a legitimate dotted path', () => { + const target = { initialView: { yaw: 0, pitch: 0, fov: 75 } }; + $exeDevice.setNestedPath(target, 'initialView.yaw', 90); + expect(target.initialView.yaw).toBe(90); + }); + + it('assigns a top-level key for a single-segment path', () => { + const target = { src: '' }; + $exeDevice.setNestedPath(target, 'src', 'asset://pano.jpg'); + expect(target.src).toBe('asset://pano.jpg'); + }); + + it('does not pollute Object.prototype via a __proto__ payload', () => { + const payload = JSON.parse('{"__proto__":{"polluted":1}}'); + $exeDevice.setNestedPath({}, '__proto__.polluted', payload.__proto__.polluted); + expect({}.polluted).toBeUndefined(); + expect(Object.prototype.polluted).toBeUndefined(); + }); + + it('ignores assignments through constructor / prototype keys', () => { + $exeDevice.setNestedPath({}, 'constructor.prototype.polluted', 1); + $exeDevice.setNestedPath({}, 'prototype.polluted', 1); + expect({}.polluted).toBeUndefined(); + expect(Object.prototype.polluted).toBeUndefined(); + }); + }); }); diff --git a/public/files/perm/idevices/base/trivial/edition/trivial.js b/public/files/perm/idevices/base/trivial/edition/trivial.js index d569200d11..92bb0a5ff6 100644 --- a/public/files/perm/idevices/base/trivial/edition/trivial.js +++ b/public/files/perm/idevices/base/trivial/edition/trivial.js @@ -3113,6 +3113,24 @@ var $exeDevice = { return valids > 0 ? $exeDevice.selectsGame : false; }, + /** + * Strip HTML tags from a string. + * Applies the tag-removal regex repeatedly until the string stops + * changing, so that obfuscated/nested payloads (e.g. "ipt>") + * cannot reconstitute a tag after a single pass. + * @param {string} text + * @returns {string} + */ + stripHtmlTags: function (text) { + let result = String(text == null ? '' : text); + let previous; + do { + previous = result; + result = result.replace(/<[^>]*>/g, ''); + } while (result !== previous); + return result; + }, + importGlosary: function (xmlText) { const parser = new DOMParser(), xmlDoc = parser.parseFromString(xmlText, 'text/xml'), @@ -3128,10 +3146,9 @@ var $exeDevice = { $entries.find('ENTRY').each(function () { var $this = $(this), concept = $this.find('CONCEPT').text(), - definition = $this - .find('DEFINITION') - .text() - .replace(/<[^>]*>/g, ''); // Elimina HTML + definition = $exeDevice.stripHtmlTags( + $this.find('DEFINITION').text() + ); // Elimina HTML if (concept && definition) { questionsJson.push({ solution: concept, diff --git a/public/files/perm/idevices/base/trivial/edition/trivial.test.js b/public/files/perm/idevices/base/trivial/edition/trivial.test.js index e30f0334ec..ac3667f9d3 100644 --- a/public/files/perm/idevices/base/trivial/edition/trivial.test.js +++ b/public/files/perm/idevices/base/trivial/edition/trivial.test.js @@ -1,9 +1,12 @@ /** - * Unit tests for the trivial iDevice editor. + * Unit tests for trivial iDevice (edition) * - * The numeric fields truncate their value on keyup. Capping them at one digit - * made ordinary values impossible to enter: the second keystroke was dropped, - * so an author aiming for 10 silently ended up with 1. + * Covers: + * - stripHtmlTags: HTML-tag sanitizer used when importing glossary entries. + * It must remove tags even from obfuscated/nested payloads that would + * survive a single-pass strip. + * - Numeric field limits: the silence-time field truncates on keyup; capping + * it at one digit made ordinary values impossible to enter. */ /* eslint-disable no-undef */ @@ -13,6 +16,52 @@ import { dirname, join } from 'path'; const __filename = fileURLToPath(import.meta.url); const __dirname = dirname(__filename); +describe('trivial iDevice (edition)', () => { + let $exeDevice; + + beforeEach(() => { + global.$exeDevice = undefined; + $exeDevice = global.loadIdevice(join(__dirname, 'trivial.js')); + }); + + describe('stripHtmlTags', () => { + it('exists as a function', () => { + expect(typeof $exeDevice.stripHtmlTags).toBe('function'); + }); + + it('removes simple HTML tags but keeps text content', () => { + expect($exeDevice.stripHtmlTags('hello')).toBe('hello'); + expect($exeDevice.stripHtmlTags('a b c')).toBe('a b c'); + }); + + it('preserves plain text without tags unchanged', () => { + expect($exeDevice.stripHtmlTags('plain definition')).toBe('plain definition'); + }); + + it('strips a nested/obfuscated script payload leaving no script tag', () => { + const payload = 'ipt>alert(1)'; + const out = $exeDevice.stripHtmlTags(payload); + // No reconstituted ". + // The fixed-point loop must keep stripping until nothing remains. + const payload = 'ipt>alert(1)safe'; + $exeDevice.importGlosary(glossaryXml('term', payload)); + + expect(captured).toHaveLength(1); + const definition = captured[0].definition; + // The security property: no opening tag delimiter survives, so the + // reassembled "" left as text + // content is inert and not a tag.) + expect(definition.toLowerCase()).not.toContain(' { + let captured = null; + $exeDevice.addWords = (words) => { + captured = words; + }; + + // A naive single pass on "< Inc'; + const result = extractCopyrightFromLicense(content); + expect(result).not.toContain('<'); + expect(result?.toLowerCase()).not.toContain(' { + const content = 'Copyright (c) 2023 Acme Inc'; + expect(extractCopyrightFromLicense(content)).toBe('Acme Inc'); + const parens = 'Copyright (c) 2023 Acme (legacy) (note) Inc'; + expect(extractCopyrightFromLicense(parens)).toBe('Acme Inc'); + }); + it('should ignore the Apache-2.0 grant of copyright license', () => { // Section 2 of the verbatim text. It is what pdfjs-dist, @material-symbols/svg-400 // and @mathjax/src would otherwise be attributed to, and generation and --check diff --git a/src/cli/commands/update-licenses.ts b/src/cli/commands/update-licenses.ts index 071fc59f18..a32d9c635e 100644 --- a/src/cli/commands/update-licenses.ts +++ b/src/cli/commands/update-licenses.ts @@ -255,10 +255,19 @@ export function extractCopyrightFromLicense(content: string): string | null { // Get first line only let author = match[1].split('\n')[0]; // Clean up the result - remove "All rights reserved", email, etc. - author = author - .replace(/all rights reserved\.?/gi, '') - .replace(/^[-*•]+\s*/, '') // Drop a list bullet: `yjs` lists its holders - .replace(/,?\s*as listed in:.*$/i, ''); // Drop pointers to a contributors page + // Apply the strip passes repeatedly to a fixed point: removing one + // <...> or (...) match can splice two halves into a brand-new match + // (e.g. "c>"), so a single pass is insufficient. + let prev: string; + do { + prev = author; + author = author + .replace(/all rights reserved\.?/gi, '') + .replace(/^[-*•]+\s*/, '') // Drop a list bullet: `yjs` lists its holders + .replace(/,?\s*as listed in:.*$/i, '') // Drop pointers to a contributors page + .replace(/<[^>]+>/g, '') // Remove emails in + .replace(/\s*\([^)]*\)/g, ''); // Remove parenthetical notes + } while (author !== prev); // Drop trailing URLs, and with them the preposition that introduced one: // `fast-uri` writes `Gary Court until `, which otherwise ends as @@ -278,8 +287,6 @@ export function extractCopyrightFromLicense(content: string): string | null { ).replace(/[\s,]+(?:and|&)$/i, ''); author = author - .replace(/<[^>]+>/g, '') // Remove emails in - .replace(/\s*\([^)]*\)/g, '') // Remove parenthetical notes .replace(/\s+/g, ' ') // Normalize whitespace .trim(); // Remove trailing punctuation diff --git a/src/routes/project.spec.ts b/src/routes/project.spec.ts index b9e0ead430..c7a7f61b84 100644 --- a/src/routes/project.spec.ts +++ b/src/routes/project.spec.ts @@ -2169,7 +2169,7 @@ describe('Project Routes', () => { expect(body.brokenLinks[0].brokenLinks).toBe('No broken links found'); }); - it('should skip javascript and data URLs', async () => { + it('should skip javascript, vbscript and data URLs (case-insensitive, padded)', async () => { const res = await app.handle( new Request('http://localhost/api/ode-management/odes/session/brokenlinks', { method: 'POST', @@ -2177,7 +2177,14 @@ describe('Project Routes', () => { body: JSON.stringify({ idevices: [ { - html: 'JS Link', + html: + 'JS Link' + + 'Mixed JS' + + 'Padded JS' + + 'VBScript' + + 'Mixed VBScript' + + '' + + '', }, ], }), @@ -2186,6 +2193,7 @@ describe('Project Routes', () => { expect(res.status).toBe(200); const body = await res.json(); + // Every dangerous-scheme link is filtered out, so nothing is reported. expect(body.brokenLinks[0].brokenLinks).toBe('No broken links found'); }); }); diff --git a/src/routes/project.ts b/src/routes/project.ts index d7cabf2bfa..4c96677a27 100644 --- a/src/routes/project.ts +++ b/src/routes/project.ts @@ -98,6 +98,7 @@ export async function resolveDefaultThemeForNewProject( } return themeDirName; } + import { getAppVersion } from '../utils/version'; import { buildSiteThemeUrl } from '../utils/site-theme-url'; import { isSafePathSegment, isWithinBase } from '../utils/safe-path'; diff --git a/src/services/folder-manager.spec.ts b/src/services/folder-manager.spec.ts index 1333944d91..da50579a28 100644 --- a/src/services/folder-manager.spec.ts +++ b/src/services/folder-manager.spec.ts @@ -148,6 +148,19 @@ describe('Folder Manager Service', () => { expect(service.sanitizeFolderName('folder*name')).toBe('folder_name'); }); + it('should replace ALL invalid characters, not just the first', () => { + // Incomplete-sanitization regression guard: without the global flag + // only the first invalid character would be replaced, leaving the + // rest (e.g. directory separators) intact. + expect(service.sanitizeFolderName('ac')).toBe('a_b_c'); + expect(service.sanitizeFolderName('a/b/c')).toBe('a_b_c'); + expect(service.sanitizeFolderName('a\\b\\c')).toBe('a_b_c'); + expect(service.sanitizeFolderName('<>:"|?*')).toBe('_______'); + // No invalid character should survive sanitization. + expect(service.sanitizeFolderName('x:y/z')).not.toContain('/'); + expect(service.sanitizeFolderName('x:y/z')).not.toContain(':'); + }); + it('should remove leading dots and spaces', () => { expect(service.sanitizeFolderName('.hidden')).toBe('hidden'); expect(service.sanitizeFolderName(' folder')).toBe('folder'); diff --git a/src/services/folder-manager.ts b/src/services/folder-manager.ts index a1428f05b0..4da0fc577c 100644 --- a/src/services/folder-manager.ts +++ b/src/services/folder-manager.ts @@ -118,6 +118,9 @@ export interface FolderManagerService { // Invalid characters for folder names (cross-platform safe) // Note: we check for control chars (0-31) separately to avoid regex escape issues const INVALID_CHARS = /[<>:"|?*\\/]/; +// Global variant used for sanitization so that ALL invalid characters are +// replaced, not just the first occurrence (incomplete sanitization fix). +const INVALID_CHARS_GLOBAL = /[<>:"|?*\\/]/g; const RESERVED_NAMES = ['CON', 'PRN', 'AUX', 'NUL', 'COM1', 'COM2', 'COM3', 'COM4', 'LPT1', 'LPT2', 'LPT3', 'LPT4']; /** @@ -188,8 +191,8 @@ export function createFolderManagerService(deps: FolderManagerDeps = {}): Folder const sanitizeFolderName = (name: string): string => { // Remove control characters first let sanitized = removeControlChars(name); - // Remove invalid characters - sanitized = sanitized.replace(INVALID_CHARS, '_'); + // Remove invalid characters (global: replace every occurrence) + sanitized = sanitized.replace(INVALID_CHARS_GLOBAL, '_'); // Remove leading/trailing dots and spaces sanitized = sanitized.replace(/^[.\s]+|[.\s]+$/g, ''); // Truncate to max length diff --git a/src/services/link-validator.spec.ts b/src/services/link-validator.spec.ts index 8e33633165..3f2dd48b0f 100644 --- a/src/services/link-validator.spec.ts +++ b/src/services/link-validator.spec.ts @@ -133,6 +133,101 @@ describe('Link Validator Service', () => { const result = removeInvalidLinks(links); expect(result).toHaveLength(0); }); + + it('should remove vbscript: links', () => { + const links: RawExtractedLink[] = [ + { url: 'vbscript:msgbox("hi")', count: 1 }, + { url: 'VBScript:Execute("x")', count: 1 }, + ]; + const result = removeInvalidLinks(links); + expect(result).toHaveLength(0); + }); + + it('should remove dangerous schemes regardless of case', () => { + const links: RawExtractedLink[] = [ + { url: 'JaVaScRiPt:alert(1)', count: 1 }, + { url: 'JAVASCRIPT:void(0)', count: 1 }, + { url: 'DATA:text/html,

Hi

', count: 1 }, + ]; + const result = removeInvalidLinks(links); + expect(result).toHaveLength(0); + }); + + it('should remove dangerous schemes with leading whitespace/control chars', () => { + const links: RawExtractedLink[] = [ + { url: ' javascript:alert(1)', count: 1 }, + { url: '\tjavascript:alert(2)', count: 1 }, + { url: '\n\rvbscript:msgbox(1)', count: 1 }, + { url: ' data:text/html,

Hi

', count: 1 }, + ]; + const result = removeInvalidLinks(links); + expect(result).toHaveLength(0); + }); + + it('should remove dangerous schemes hidden by embedded control chars', () => { + const links: RawExtractedLink[] = [ + { url: 'java\tscript:alert(1)', count: 1 }, + { url: 'java\nscript:alert(2)', count: 1 }, + ]; + const result = removeInvalidLinks(links); + expect(result).toHaveLength(0); + }); + + it('should remove dangerous schemes obfuscated with HTML entities', () => { + const links: RawExtractedLink[] = [ + { url: 'java script:alert(1)', count: 1 }, + { url: 'java script:alert(2)', count: 1 }, + { url: 'java script:alert(3)', count: 1 }, + ]; + const result = removeInvalidLinks(links); + expect(result).toHaveLength(0); + }); + + it('should remove schemes whose own letters/colon are HTML-entity encoded', () => { + // The old normalizer only decoded whitespace entities, so these + // (the scheme characters themselves encoded) slipped through. They + // must be caught to match the client-side LinkValidationAdapter. + const links: RawExtractedLink[] = [ + { url: 'javascript:alert(1)', count: 1 }, // 'j' as j + { url: 'javascript:alert(2)', count: 1 }, // 'j' as j + { url: 'javascript:alert(3)', count: 1 }, // ':' as : + { url: 'javascript:alert(4)', count: 1 }, // ':' as : + { url: 'data:text/html,evil', count: 1 }, + ]; + const result = removeInvalidLinks(links); + expect(result).toHaveLength(0); + }); + + it('should not throw on out-of-range numeric entities and keep the benign link', () => { + // Out-of-range code points must not crash String.fromCodePoint; the + // junk entity is dropped, so the benign link survives while the + // still-dangerous one is removed. + const links: RawExtractedLink[] = [ + { url: 'https://example.com/�page', count: 1 }, + { url: '�javascript:alert(1)', count: 1 }, + ]; + const result = removeInvalidLinks(links); + expect(result.map(l => l.url)).toEqual(['https://example.com/�page']); + }); + + it('should keep legitimate http/https/relative links', () => { + const links: RawExtractedLink[] = [ + { url: 'http://example.com', count: 1 }, + { url: 'https://example.com/page', count: 1 }, + { url: 'files/image.png', count: 1 }, + { url: 'mailto:user@example.com', count: 1 }, + { url: '//cdn.example.com/lib.js', count: 1 }, + ]; + const result = removeInvalidLinks(links); + expect(result).toHaveLength(5); + expect(result.map(l => l.url)).toEqual([ + 'http://example.com', + 'https://example.com/page', + 'files/image.png', + 'mailto:user@example.com', + '//cdn.example.com/lib.js', + ]); + }); }); describe('deduplicateLinks', () => { diff --git a/src/services/link-validator.ts b/src/services/link-validator.ts index 2a8682da3d..456400d932 100644 --- a/src/services/link-validator.ts +++ b/src/services/link-validator.ts @@ -84,16 +84,67 @@ export function cleanAndCountLinks(links: RawExtractedLink[]): RawExtractedLink[ return Array.from(urlCounts.entries()).map(([url, count]) => ({ url, count })); } +/** + * Schemes that can execute script or smuggle markup. These must never be + * treated as validatable links, regardless of casing, leading whitespace, + * control characters, or HTML-entity obfuscation. + */ +const DANGEROUS_URL_SCHEMES = ['javascript:', 'vbscript:', 'data:']; + +/** + * Normalize a URL for scheme inspection so an encoded or padded scheme cannot + * slip past the dangerous-scheme filter. Decodes HTML entities (named and + * numeric - e.g. "javascript:", "javascript:", "data:..."), + * then strips the whitespace and C0/DEL control characters that browsers ignore + * when resolving a URL scheme, and lowercases the result. This prevents bypasses + * such as " javascript:", "JaVaScRiPt:", "java script:", or "javascript:". + * + * Kept in sync with the client-side `LinkValidationAdapter._hasDangerousScheme()` + * so the broken-link checker rejects the same set of links on both ends. + */ +function normalizeUrlForSchemeCheck(url: string): string { + // Safely convert a numeric code point, returning '' for invalid or + // out-of-range values so String.fromCodePoint cannot throw a RangeError + // (e.g. on "�" or "�"). Dropping such junk only makes + // scheme detection stricter, never weaker. + const safeFromCodePoint = (cp: number): string => + Number.isInteger(cp) && cp >= 0 && cp <= 0x10ffff ? String.fromCodePoint(cp) : ''; + + const namedEntities: Record = { + amp: '&', + lt: '<', + gt: '>', + quot: '"', + apos: "'", + colon: ':', + tab: '\t', + newline: '\n', + }; + + const decoded = url + .replace(/&#x([0-9a-f]+);?/gi, (_m, hex: string) => safeFromCodePoint(Number.parseInt(hex, 16))) + .replace(/&#(\d+);?/g, (_m, dec: string) => safeFromCodePoint(Number.parseInt(dec, 10))) + .replace( + /&(amp|lt|gt|quot|apos|colon|tab|newline);/gi, + (_m, name: string) => namedEntities[name.toLowerCase()] ?? _m, + ); + + // Strip whitespace and control characters (incl. NUL and DEL) that browsers + // ignore when resolving the scheme, then lowercase for comparison. + // biome-ignore lint/suspicious/noControlCharactersInRegex: intentionally matching the C0/DEL control chars browsers ignore when resolving a URL scheme, to block obfuscated-scheme bypasses. + return decoded.replace(/[\u0000-\u0020\u007f]+/g, '').toLowerCase(); +} + /** * Remove invalid/non-validatable links - * Filters out: empty, anchors (#), javascript:, data: URLs + * Filters out: empty, anchors (#), and dangerous schemes (javascript:, vbscript:, data:) */ export function removeInvalidLinks(links: RawExtractedLink[]): RawExtractedLink[] { return links.filter(link => { if (!link.url || link.url.trim() === '') return false; if (link.url.startsWith('#')) return false; - if (link.url.startsWith('javascript:')) return false; - if (link.url.startsWith('data:')) return false; + const normalized = normalizeUrlForSchemeCheck(link.url); + if (DANGEROUS_URL_SCHEMES.some(scheme => normalized.startsWith(scheme))) return false; return true; }); } diff --git a/src/shared/export/exporters/PrintPreviewExporter.spec.ts b/src/shared/export/exporters/PrintPreviewExporter.spec.ts index 4aa45a5f8a..38b7e1020c 100644 --- a/src/shared/export/exporters/PrintPreviewExporter.spec.ts +++ b/src/shared/export/exporters/PrintPreviewExporter.spec.ts @@ -786,33 +786,7 @@ describe('PrintPreviewExporter', () => { const exp = new PrintPreviewExporter(doc, mockResourceProvider); const result = await exp.generatePreview(); - // Helper to check for display: none !important - const checkHidden = (snippet: string) => { - // Determine the tag type to construct the expectation - const isImg = snippet.startsWith('', '.*style=.*display:\\s*none.*!important.*>')), - ); - }; - - // Can't easily use regex match on exact input string because attributes might move if we parsed them? - // If we use string replace, attributes stay mostly put. - // Let's check that the specific class combinations now have style="display: none !important" attached. + // Check that the specific class combinations now have style="display: none !important" attached. // 1. Version divs expect(result.html).toContain('class="sopa-version js-hidden" style="display: none !important"'); diff --git a/src/shared/export/generators/I18nGenerator.spec.ts b/src/shared/export/generators/I18nGenerator.spec.ts index 00b2d2ad27..32d2ab843c 100644 --- a/src/shared/export/generators/I18nGenerator.spec.ts +++ b/src/shared/export/generators/I18nGenerator.spec.ts @@ -80,6 +80,20 @@ describe('parseXlfTranslations', () => { expect(map.get('Entities & test')).toBe('Entidades & prueba'); }); + it('should not double-decode escaped entities (&lt; stays <, &amp; stays &)', () => { + // Security: decoding & -> & must happen LAST so that the literal text + // "<" (written in XML as "&lt;") does not wrongly collapse to "<". + const xlf = ` + + literal lt&lt; + literal amp&amp; + +`; + const map = parseXlfTranslations(xlf); + expect(map.get('literal lt')).toBe('<'); + expect(map.get('literal amp')).toBe('&'); + }); + it('should return an empty Map for an empty XLF string', () => { const map = parseXlfTranslations(''); expect(map.size).toBe(0); diff --git a/src/shared/export/generators/I18nGenerator.ts b/src/shared/export/generators/I18nGenerator.ts index 0a70f67328..8d639b517d 100644 --- a/src/shared/export/generators/I18nGenerator.ts +++ b/src/shared/export/generators/I18nGenerator.ts @@ -17,13 +17,13 @@ */ function decodeXmlEntities(str: string): string { return str - .replace(/&/g, '&') .replace(/</g, '<') .replace(/>/g, '>') .replace(/"/g, '"') .replace(/'/g, "'") .replace(/&#(\d+);/g, (_, num) => String.fromCharCode(parseInt(num, 10))) - .replace(/&#x([0-9a-fA-F]+);/g, (_, hex) => String.fromCharCode(parseInt(hex, 16))); + .replace(/&#x([0-9a-fA-F]+);/g, (_, hex) => String.fromCharCode(parseInt(hex, 16))) + .replace(/&/g, '&'); } /** diff --git a/src/shared/export/prerender/ServerLatexPreRenderer.spec.ts b/src/shared/export/prerender/ServerLatexPreRenderer.spec.ts index 9287903b8a..f7e9cbdcb7 100644 --- a/src/shared/export/prerender/ServerLatexPreRenderer.spec.ts +++ b/src/shared/export/prerender/ServerLatexPreRenderer.spec.ts @@ -3,7 +3,7 @@ */ import { describe, it, expect, beforeAll } from 'bun:test'; -import { ServerLatexPreRenderer } from './ServerLatexPreRenderer'; +import { ServerLatexPreRenderer, cleanLatexFromHtml } from './ServerLatexPreRenderer'; describe('ServerLatexPreRenderer', () => { let renderer: ServerLatexPreRenderer; @@ -302,6 +302,44 @@ describe('ServerLatexPreRenderer', () => { }); }); + describe('cleanLatexFromHtml', () => { + it('should strip nested/obfuscated tags without leaving a reassembled tag', () => { + // A naive single pass on this payload removes the inner tag and splices + // the halves into a NEW "