fix(llm-security): out-of-range char-ref silently emptied a plugin.xml field
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 <name> 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcQyMTQfyrsAapaCMPxTtQ
This commit is contained in:
parent
4f21374e8c
commit
a035455979
2 changed files with 69 additions and 2 deletions
|
|
@ -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]
|
||||
|
|
|
|||
59
tests/scanners/ide-extension-parser-entities.test.mjs
Normal file
59
tests/scanners/ide-extension-parser-entities.test.mjs
Normal file
|
|
@ -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 `<idea-plugin><id>com.example.p</id><name>${name}</name><vendor>v</vendor></idea-plugin>`;
|
||||
}
|
||||
|
||||
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<C');
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue