JS-831 Improve detection of S1523 and S2755 (#5951)

loris-s-sonarsource · web-flow · commit ae4d2a6bbe6c · 2025-11-20T09:37:20.000+01:00
diff --git a/its/ruling/src/test/expected/jsts/TypeScript/typescript-S1523.json b/its/ruling/src/test/expected/jsts/TypeScript/typescript-S1523.json
@@ -1,9 +1,12 @@
 {
 "TypeScript:src/harness/fourslash.ts": [
 588,
-2886
+2886,
+2887
 ],
 "TypeScript:src/harness/harness.ts": [
-87
+87,
+92,
+95
 ]
 }
diff --git a/its/ruling/src/test/expected/jsts/ace/javascript-S1523.json b/its/ruling/src/test/expected/jsts/ace/javascript-S1523.json
@@ -1,6 +1,7 @@
 {
 "ace:demo/kitchen-sink/dev_util.js": [
-166
+166,
+167
 ],
 "ace:experiments/worker.js": [
 2
diff --git a/its/ruling/src/test/expected/jsts/es5-shim/javascript-S1523.json b/its/ruling/src/test/expected/jsts/es5-shim/javascript-S1523.json
@@ -1,5 +1,8 @@
 {
 "es5-shim:es5-sham.js": [
 255
+],
+"es5-shim:es5-shim.js": [
+302
 ]
 }
diff --git a/its/ruling/src/test/expected/jsts/file-for-rules/javascript-S1451.json b/its/ruling/src/test/expected/jsts/file-for-rules/javascript-S1451.json
@@ -5,6 +5,9 @@
 "file-for-rules:S1291.js": [
 0
 ],
+"file-for-rules:S1523.js": [
+0
+],
 "file-for-rules:S1534.js": [
 0
 ],
@@ -23,6 +26,9 @@
 "file-for-rules:S2639.js": [
 0
 ],
+"file-for-rules:S2755.js": [
+0
+],
 "file-for-rules:S2871.js": [
 0
 ],
diff --git a/its/ruling/src/test/expected/jsts/file-for-rules/javascript-S1523.json b/its/ruling/src/test/expected/jsts/file-for-rules/javascript-S1523.json
@@ -0,0 +1,5 @@
+{
+"file-for-rules:S1523.js": [
+5
+]
+}
diff --git a/its/ruling/src/test/expected/jsts/file-for-rules/javascript-S2755.json b/its/ruling/src/test/expected/jsts/file-for-rules/javascript-S2755.json
@@ -0,0 +1,5 @@
+{
+"file-for-rules:S2755.js": [
+8
+]
+}
diff --git a/its/ruling/src/test/expected/jsts/redux/javascript-S1523.json b/its/ruling/src/test/expected/jsts/redux/javascript-S1523.json
@@ -0,0 +1,5 @@
+{
+"redux:test/utils/isPlainObject.spec.js": [
+12
+]
+}
diff --git a/its/ruling/src/test/expected/jsts/sizzle/javascript-S1523.json b/its/ruling/src/test/expected/jsts/sizzle/javascript-S1523.json
@@ -1,5 +1,6 @@
 {
 "sizzle:speed/speed.js": [
-371
+371,
+378
 ]
 }
diff --git a/its/sources/jsts/custom/S1523.js b/its/sources/jsts/custom/S1523.js
@@ -0,0 +1,5 @@
+import vm from 'node:vm';
+
+const sandbox = { data: 'data' };
+vm.createContext(sandbox);
+vm.runInContext('code', sandbox, { timeout: 2000 });
diff --git a/its/sources/jsts/custom/S2755.js b/its/sources/jsts/custom/S2755.js
@@ -0,0 +1,9 @@
+import libxmljs from 'libxmljs';
+import libxmljs2 from 'libxmljs2';
+import fs from 'node:fs';
+
+const xml = fs.readFileSync('xxe.xml', 'utf8');
+const opts = { noblanks: true, noent: true, nocdata: true };
+
+libxmljs.parseXmlString(xml, { noblanks: true, noent: true, nocdata: true }); // Detected
+libxmljs2.parseXmlString(xml, opts); // Undetected
diff --git a/packages/jsts/src/rules/S1523/rule.ts b/packages/jsts/src/rules/S1523/rule.ts
@@ -20,11 +20,21 @@
 import type { Rule } from 'eslint';
 import type estree from 'estree';
 import { getESLintCoreRule } from '../external/core.js';
-import { generateMeta } from '../helpers/index.js';
+import { generateMeta, getFullyQualifiedName } from '../helpers/index.js';
 import * as meta from './generated-meta.js';
 
 const noScriptUrlRule = getESLintCoreRule('no-script-url');
 
+const EVAL_LIKE_FUNCTIONS: Set<string> = new Set([
+  'eval',
+  'Function',
+  'vm.Script',
+  'vm.SourceTextModule',
+  'vm.runInContext',
+  'vm.runInNewContext',
+  'vm.runInThisContext',
+]);
+
 export const rule: Rule.RuleModule = {
   meta: generateMeta(meta, {
     messages: {
@@ -44,9 +54,9 @@ export const rule: Rule.RuleModule = {
 };
 
 function checkCallExpression(node: estree.CallExpression, context: Rule.RuleContext) {
-  if (node.callee.type === 'Identifier') {
-    const { name } = node.callee;
-    if ((name === 'eval' || name === 'Function') && hasAtLeastOneVariableArgument(node.arguments)) {
+  if (['Identifier', 'MemberExpression'].includes(node.callee.type)) {
+    const name = getFullyQualifiedName(context, node) || '';
+    if (EVAL_LIKE_FUNCTIONS.has(name) && hasAtLeastOneVariableArgument(node.arguments)) {
       context.report({
         messageId: 'safeCode',
         node: node.callee,
diff --git a/packages/jsts/src/rules/S2755/rule.ts b/packages/jsts/src/rules/S2755/rule.ts
@@ -28,15 +28,19 @@ import {
 } from '../helpers/index.js';
 import * as meta from './generated-meta.js';
 
-const XML_LIBRARY = 'libxmljs';
-const XML_PARSERS = ['parseXml', 'parseXmlString'];
+const TARGET_XML_FQNS: Set<string> = new Set([
+  'libxmljs.parseXml',
+  'libxmljs.parseXmlString',
+  'libxmljs2.parseXml',
+  'libxmljs2.parseXmlString',
+]);
 
 export const rule: Rule.RuleModule = {
   meta: generateMeta(meta),
   create(context: Rule.RuleContext) {
     function isXmlParserCall(call: estree.CallExpression) {
-      const fqn = getFullyQualifiedName(context, call);
-      return XML_PARSERS.some(parser => fqn === `${XML_LIBRARY}.${parser}`);
+      const fqn = getFullyQualifiedName(context, call) || '';
+      return TARGET_XML_FQNS.has(fqn);
     }
 
     return {

Original file line number	Diff line number	Diff line change
`@@ -1,9 +1,12 @@`
`1`	`1`	`{`
`2`	`2`	`"TypeScript:src/harness/fourslash.ts": [`
`3`	`3`	`588,`
`4`		`-2886`
	`4`	`+2886,`
	`5`	`+2887`
`5`	`6`	`],`
`6`	`7`	`"TypeScript:src/harness/harness.ts": [`
`7`		`-87`
	`8`	`+87,`
	`9`	`+92,`
	`10`	`+95`
`8`	`11`	`]`
`9`	`12`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,7 @@`
`1`	`1`	`{`
`2`	`2`	`"ace:demo/kitchen-sink/dev_util.js": [`
`3`		`-166`
	`3`	`+166,`
	`4`	`+167`
`4`	`5`	`],`
`5`	`6`	`"ace:experiments/worker.js": [`
`6`	`7`	`2`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,8 @@`
`1`	`1`	`{`
`2`	`2`	`"es5-shim:es5-sham.js": [`
`3`	`3`	`255`
	`4`	`+],`
	`5`	`+"es5-shim:es5-shim.js": [`
	`6`	`+302`
`4`	`7`	`]`
`5`	`8`	`}`
Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,9 @@`
`5`	`5`	`"file-for-rules:S1291.js": [`
`6`	`6`	`0`
`7`	`7`	`],`
	`8`	`+"file-for-rules:S1523.js": [`
	`9`	`+0`
	`10`	`+],`
`8`	`11`	`"file-for-rules:S1534.js": [`
`9`	`12`	`0`
`10`	`13`	`],`
`@@ -23,6 +26,9 @@`
`23`	`26`	`"file-for-rules:S2639.js": [`
`24`	`27`	`0`
`25`	`28`	`],`
	`29`	`+"file-for-rules:S2755.js": [`
	`30`	`+0`
	`31`	`+],`
`26`	`32`	`"file-for-rules:S2871.js": [`
`27`	`33`	`0`
`28`	`34`	`],`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +{
 +"file-for-rules:S1523.js": [
 +5
 +]
 +}
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"sizzle:speed/speed.js": [`
`3`		`-371`
	`3`	`+371,`
	`4`	`+378`
`4`	`5`	`]`
`5`	`6`	`}`