From a03545597924163150a343c75b5d906dadff6379 Mon Sep 17 00:00:00 2001 From: Kjell Tore Guttormsen Date: Sat, 18 Jul 2026 09:21:56 +0200 Subject: [PATCH] fix(llm-security): out-of-range char-ref silently emptied a plugin.xml field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit decodeEntities guarded parseInt with Number.isFinite, which bounds nothing: any numeric character reference above 0x10FFFF produced a finite integer that String.fromCodePoint rejects with RangeError. Correction to the review's framing: this does NOT break parsePluginXml's no-throw contract. The per-field safe() wrapper catches it. The actual defect is quieter — the affected field is discarded and replaced with '', with only a warning. A plugin whose carries one out-of-range reference parses as name: "" while pluginId and every other field survive, so name-based checks (JetBrains typosquat detection against the top-plugin list) run against an empty string. Severity is below the HIGH it was filed as: a document containing a code point above 0x10FFFF is not well-formed XML, so IntelliJ would reject such a plugin too — the evasion yields a plugin that does not load. The fix is still correct and one line of logic. - isDecodableCodePoint(): integer, >= 0, <= 0x10FFFF. - Undecodable references are now left literal, matching how lenient parsers treat unrecognised entities and how this same function already treats unknown named entities. Regression test drives parsePluginXml with hex and decimal references just past and far past the maximum, asserts the no-throw contract still holds, and pins that valid references, the maximum valid code point, and named entities still decode. npm test: 1901/1901 green (1890 + 11 new). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01TcQyMTQfyrsAapaCMPxTtQ --- scanners/lib/ide-extension-parser.mjs | 12 +++- .../ide-extension-parser-entities.test.mjs | 59 +++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 tests/scanners/ide-extension-parser-entities.test.mjs diff --git a/scanners/lib/ide-extension-parser.mjs b/scanners/lib/ide-extension-parser.mjs index a6c0873..8bc5f5b 100644 --- a/scanners/lib/ide-extension-parser.mjs +++ b/scanners/lib/ide-extension-parser.mjs @@ -140,15 +140,23 @@ const NAMED_ENTITIES = { * @param {string} s * @returns {string} */ +/** Unicode's maximum code point — String.fromCodePoint throws RangeError above it. */ +const MAX_CODE_POINT = 0x10FFFF; + +/** Number.isFinite does not bound the value; fromCodePoint needs an actual range check. */ +function isDecodableCodePoint(cp) { + return Number.isInteger(cp) && cp >= 0 && cp <= MAX_CODE_POINT; +} + function decodeEntities(s) { return s.replace(/&(#x?[0-9a-fA-F]+|[a-zA-Z]+);/g, (full, inner) => { if (inner.startsWith('#x') || inner.startsWith('#X')) { const cp = parseInt(inner.slice(2), 16); - return Number.isFinite(cp) ? String.fromCodePoint(cp) : full; + return isDecodableCodePoint(cp) ? String.fromCodePoint(cp) : full; } if (inner.startsWith('#')) { const cp = parseInt(inner.slice(1), 10); - return Number.isFinite(cp) ? String.fromCodePoint(cp) : full; + return isDecodableCodePoint(cp) ? String.fromCodePoint(cp) : full; } return Object.prototype.hasOwnProperty.call(NAMED_ENTITIES, inner) ? NAMED_ENTITIES[inner] diff --git a/tests/scanners/ide-extension-parser-entities.test.mjs b/tests/scanners/ide-extension-parser-entities.test.mjs new file mode 100644 index 0000000..3ae479c --- /dev/null +++ b/tests/scanners/ide-extension-parser-entities.test.mjs @@ -0,0 +1,59 @@ +// ide-extension-parser-entities.test.mjs — Regression tests for XML numeric +// character references in plugin.xml. +// +// parsePluginXml documents a no-throw contract: "Malformed input returns +// { manifest: null, warnings: [...] } rather than throwing." decodeEntities +// guarded parseInt with Number.isFinite, which does not bound the value, so +// String.fromCodePoint raised RangeError for any code point above 0x10FFFF — +// a one-token hostile plugin.xml that aborts the parse instead of being +// reported. + +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { parsePluginXml } from '../../scanners/lib/ide-extension-parser.mjs'; + +function xmlWithName(name) { + return `com.example.p${name}v`; +} + +describe('ide-extension-parser — numeric character references', () => { + const outOfRange = [ + ['hex, just past the Unicode maximum', '�'], + ['decimal, just past the Unicode maximum', '�'], + ['hex, far out of range', '�'], + ['decimal, far out of range', '�'], + ]; + + for (const [label, ref] of outOfRange) { + it(`does not throw on an out-of-range reference (${label})`, () => { + assert.doesNotThrow(() => parsePluginXml(xmlWithName(`Bad${ref}Name`))); + }); + + it(`leaves an out-of-range reference literal (${label})`, () => { + const { manifest } = parsePluginXml(xmlWithName(`Bad${ref}Name`)); + assert.ok(manifest, 'manifest should still parse'); + assert.ok( + manifest.name.includes(ref), + `undecodable reference should be left as-is, got: ${manifest.name}` + ); + }); + } + + it('still decodes valid numeric references', () => { + const { manifest } = parsePluginXml(xmlWithName('AB')); + assert.ok(manifest); + assert.equal(manifest.name, 'AB'); + }); + + it('still decodes the maximum valid code point', () => { + const { manifest } = parsePluginXml(xmlWithName('x􏿿')); + assert.ok(manifest); + assert.equal(manifest.name, 'x\u{10FFFF}'); + }); + + it('still decodes named entities', () => { + const { manifest } = parsePluginXml(xmlWithName('A&B<C')); + assert.ok(manifest); + assert.equal(manifest.name, 'A&B