From 92084dd74f5a6b69c3d4ce8b41d434eaec2993fb Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Wed, 11 Jun 2025 11:43:58 +0200 Subject: [PATCH 1/7] JS: add `js/template-syntax-in-string-literal` to the Code Quality suite. --- .../query-suite/javascript-code-quality.qls.expected | 1 + .../ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/javascript/ql/integration-tests/query-suite/javascript-code-quality.qls.expected b/javascript/ql/integration-tests/query-suite/javascript-code-quality.qls.expected index 451a8b4bf273..b6b9b0382779 100644 --- a/javascript/ql/integration-tests/query-suite/javascript-code-quality.qls.expected +++ b/javascript/ql/integration-tests/query-suite/javascript-code-quality.qls.expected @@ -2,6 +2,7 @@ ql/javascript/ql/src/Declarations/IneffectiveParameterType.ql ql/javascript/ql/src/Expressions/ExprHasNoEffect.ql ql/javascript/ql/src/Expressions/MissingAwait.ql ql/javascript/ql/src/LanguageFeatures/SpuriousArguments.ql +ql/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql ql/javascript/ql/src/Quality/UnhandledErrorInStreamPipeline.ql ql/javascript/ql/src/RegExp/DuplicateCharacterInCharacterClass.ql ql/javascript/ql/src/RegExp/RegExpAlwaysMatches.ql diff --git a/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql b/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql index f22b97795607..6e0150656e63 100644 --- a/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql +++ b/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql @@ -5,7 +5,10 @@ * @problem.severity warning * @id js/template-syntax-in-string-literal * @precision high - * @tags correctness + * @tags quality + * reliability + * correctness + * language-features */ import javascript From 861e4ee11eee33a1e2c41a5769c26606aa496dad Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 12 Jun 2025 11:15:36 +0200 Subject: [PATCH 2/7] JS: Added test cases including manual interpolation and string concatination. --- .../TemplateSyntaxInStringLiteral.expected | 4 +++ .../TemplateSyntaxInStringLiteral.js | 28 ++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.expected b/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.expected index 8abd7dc0aced..f01015e878c0 100644 --- a/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.expected +++ b/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.expected @@ -3,3 +3,7 @@ | TemplateSyntaxInStringLiteral.js:19:11:19:36 | 'global ... alVar}' | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:14:5:14:13 | globalVar | globalVar | | TemplateSyntaxInStringLiteral.js:28:15:28:21 | "${x} " | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:25:14:25:14 | x | x | | TemplateSyntaxInStringLiteral.js:42:17:42:57 | "Name: ... oobar}" | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:37:11:37:16 | foobar | foobar | +| TemplateSyntaxInStringLiteral.js:47:27:47:51 | ") ${ex ... got (" | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:45:20:45:27 | expected | expected | +| TemplateSyntaxInStringLiteral.js:47:71:47:83 | ") ${actual}" | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:45:12:45:17 | actual | actual | +| TemplateSyntaxInStringLiteral.js:62:15:62:29 | "Name: ${name}" | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:61:30:61:33 | name | name | +| TemplateSyntaxInStringLiteral.js:66:11:66:44 | "Name: ... {name}" | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:61:30:61:33 | name | name | diff --git a/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.js b/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.js index d21a662dc5e8..75d9475494af 100644 --- a/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.js +++ b/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.js @@ -40,4 +40,30 @@ function foo1() { writer.emit("Name: ${name}, Date: ${date}.", data); writer.emit("Name: ${name}, Date: ${date}, ${foobar}", data); // $ Alert - `foobar` is not in `data`. -} \ No newline at end of file +} + +function a(actual, expected, description) { + assert(false, "a", description, "expected (" + + typeof expected + ") ${expected} but got (" + typeof actual + ") ${actual}", { // $SPURIOUS:Alert + expected: expected, + actual: actual + }); +} + +function replacer(str, name) { + return str.replace("${name}", name); +} + +function replacerAll(str, name) { + return str.replaceAll("${name}", name); +} + +function manualInterpolation(name) { + let str = "Name: ${name}"; // $SPURIOUS:Alert + let result1 = replacer(str, name); + console.log(result1); + + str = "Name: ${name} and again: ${name}"; // $SPURIOUS:Alert + let result2 = replacerAll(str, name); + console.log(result2); +} From bafe7e66ad7d7948517cac60b4f3ea43c5617d7e Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 12 Jun 2025 11:18:20 +0200 Subject: [PATCH 3/7] JS: Fix template literal detection in string concatination --- .../src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql | 7 ++++--- .../TemplateSyntaxInStringLiteral.expected | 2 -- .../TemplateSyntaxInStringLiteral.js | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql b/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql index 6e0150656e63..b55d3c0d12e1 100644 --- a/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql +++ b/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql @@ -76,9 +76,10 @@ class CandidateStringLiteral extends StringLiteral { * ``` */ predicate hasObjectProvidingTemplateVariables(CandidateStringLiteral lit) { - exists(DataFlow::CallNode call, DataFlow::ObjectLiteralNode obj | - call.getAnArgument().getALocalSource() = obj and - call.getAnArgument().asExpr() = lit and + exists(DataFlow::CallNode call, DataFlow::ObjectLiteralNode obj, DataFlow::Node stringArg | + stringArg = [StringConcatenation::getRoot(lit.flow()), lit.flow()] and + stringArg = call.getAnArgument() and + obj.flowsTo(call.getAnArgument()) and forex(string name | name = lit.getAReferencedVariable() | exists(obj.getAPropertyWrite(name))) ) } diff --git a/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.expected b/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.expected index f01015e878c0..14e18888c32e 100644 --- a/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.expected +++ b/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.expected @@ -3,7 +3,5 @@ | TemplateSyntaxInStringLiteral.js:19:11:19:36 | 'global ... alVar}' | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:14:5:14:13 | globalVar | globalVar | | TemplateSyntaxInStringLiteral.js:28:15:28:21 | "${x} " | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:25:14:25:14 | x | x | | TemplateSyntaxInStringLiteral.js:42:17:42:57 | "Name: ... oobar}" | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:37:11:37:16 | foobar | foobar | -| TemplateSyntaxInStringLiteral.js:47:27:47:51 | ") ${ex ... got (" | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:45:20:45:27 | expected | expected | -| TemplateSyntaxInStringLiteral.js:47:71:47:83 | ") ${actual}" | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:45:12:45:17 | actual | actual | | TemplateSyntaxInStringLiteral.js:62:15:62:29 | "Name: ${name}" | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:61:30:61:33 | name | name | | TemplateSyntaxInStringLiteral.js:66:11:66:44 | "Name: ... {name}" | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:61:30:61:33 | name | name | diff --git a/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.js b/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.js index 75d9475494af..709ca34d85fc 100644 --- a/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.js +++ b/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.js @@ -44,7 +44,7 @@ function foo1() { function a(actual, expected, description) { assert(false, "a", description, "expected (" + - typeof expected + ") ${expected} but got (" + typeof actual + ") ${actual}", { // $SPURIOUS:Alert + typeof expected + ") ${expected} but got (" + typeof actual + ") ${actual}", { expected: expected, actual: actual }); From 923aff2439831c32d9a3aa9e1e1547c5bd6d1b8a Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 12 Jun 2025 11:35:33 +0200 Subject: [PATCH 4/7] JS: Fixed false positive on manual string interpolation. --- .../TemplateSyntaxInStringLiteral.ql | 26 ++++++++++++++++++- .../TemplateSyntaxInStringLiteral.expected | 2 -- .../TemplateSyntaxInStringLiteral.js | 4 +-- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql b/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql index b55d3c0d12e1..566a513bca00 100644 --- a/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql +++ b/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql @@ -95,12 +95,36 @@ VarDecl getDeclIn(Variable v, Scope scope, string name, CandidateTopLevel tl) { result.getTopLevel() = tl } +/** + * Tracks data flow from a string literal that may flow to a replace operation. + */ +DataFlow::SourceNode trackString(CandidateStringLiteral lit, DataFlow::TypeTracker t) { + t.start() and result = lit.flow() + or + exists(DataFlow::TypeTracker t2 | result = trackString(lit, t2).track(t2, t)) +} + +/** + * Gets a string literal that flows to a replace operation. + */ +DataFlow::SourceNode trackString(CandidateStringLiteral lit) { + result = trackString(lit, DataFlow::TypeTracker::end()) +} + +/** + * Holds if the string literal flows to a replace method call. + */ +predicate hasReplaceMethodCall(CandidateStringLiteral lit) { + trackString(lit).getAMethodCall() instanceof StringReplaceCall +} + from CandidateStringLiteral lit, Variable v, Scope s, string name, VarDecl decl where decl = getDeclIn(v, s, name, lit.getTopLevel()) and lit.getAReferencedVariable() = name and lit.isInScope(s) and not hasObjectProvidingTemplateVariables(lit) and - not lit.getStringValue() = "${" + name + "}" + not lit.getStringValue() = "${" + name + "}" and + not hasReplaceMethodCall(lit) select lit, "This string is not a template literal, but appears to reference the variable $@.", decl, v.getName() diff --git a/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.expected b/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.expected index 14e18888c32e..8abd7dc0aced 100644 --- a/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.expected +++ b/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.expected @@ -3,5 +3,3 @@ | TemplateSyntaxInStringLiteral.js:19:11:19:36 | 'global ... alVar}' | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:14:5:14:13 | globalVar | globalVar | | TemplateSyntaxInStringLiteral.js:28:15:28:21 | "${x} " | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:25:14:25:14 | x | x | | TemplateSyntaxInStringLiteral.js:42:17:42:57 | "Name: ... oobar}" | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:37:11:37:16 | foobar | foobar | -| TemplateSyntaxInStringLiteral.js:62:15:62:29 | "Name: ${name}" | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:61:30:61:33 | name | name | -| TemplateSyntaxInStringLiteral.js:66:11:66:44 | "Name: ... {name}" | This string is not a template literal, but appears to reference the variable $@. | TemplateSyntaxInStringLiteral.js:61:30:61:33 | name | name | diff --git a/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.js b/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.js index 709ca34d85fc..9e0993278c16 100644 --- a/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.js +++ b/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.js @@ -59,11 +59,11 @@ function replacerAll(str, name) { } function manualInterpolation(name) { - let str = "Name: ${name}"; // $SPURIOUS:Alert + let str = "Name: ${name}"; let result1 = replacer(str, name); console.log(result1); - str = "Name: ${name} and again: ${name}"; // $SPURIOUS:Alert + str = "Name: ${name} and again: ${name}"; let result2 = replacerAll(str, name); console.log(result2); } From 75ee649362392a4a1ed1a5f04b15403308e1f923 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 12 Jun 2025 12:11:35 +0200 Subject: [PATCH 5/7] JS: add change note --- .../ql/src/change-notes/2025-06-12-string-interpolation.md | 4 ++++ .../src/change-notes/2025-06-12-template-syntax-metadata.md | 4 ++++ 2 files changed, 8 insertions(+) create mode 100644 javascript/ql/src/change-notes/2025-06-12-string-interpolation.md create mode 100644 javascript/ql/src/change-notes/2025-06-12-template-syntax-metadata.md diff --git a/javascript/ql/src/change-notes/2025-06-12-string-interpolation.md b/javascript/ql/src/change-notes/2025-06-12-string-interpolation.md new file mode 100644 index 000000000000..446ecf0fcb2a --- /dev/null +++ b/javascript/ql/src/change-notes/2025-06-12-string-interpolation.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Fixed false positives in the `js/template-syntax-in-string-literal` query where template syntax in string concatenation and "manual string interpolation" patterns were incorrectly flagged. diff --git a/javascript/ql/src/change-notes/2025-06-12-template-syntax-metadata.md b/javascript/ql/src/change-notes/2025-06-12-template-syntax-metadata.md new file mode 100644 index 000000000000..f29f602095d9 --- /dev/null +++ b/javascript/ql/src/change-notes/2025-06-12-template-syntax-metadata.md @@ -0,0 +1,4 @@ +--- +category: queryMetadata +--- +* Added `reliability` and `language-features` tags to the `js/template-syntax-in-string-literal` query. From da5cd251bef4cf82ec9df5fbaba5e8b9e2b9c625 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 12 Jun 2025 14:25:00 +0200 Subject: [PATCH 6/7] Update javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql Co-Authored-By: Asger F <316427+asgerf@users.noreply.github.com> --- .../ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql b/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql index 566a513bca00..4dd91b143dcd 100644 --- a/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql +++ b/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql @@ -76,9 +76,8 @@ class CandidateStringLiteral extends StringLiteral { * ``` */ predicate hasObjectProvidingTemplateVariables(CandidateStringLiteral lit) { - exists(DataFlow::CallNode call, DataFlow::ObjectLiteralNode obj, DataFlow::Node stringArg | - stringArg = [StringConcatenation::getRoot(lit.flow()), lit.flow()] and - stringArg = call.getAnArgument() and + exists(DataFlow::CallNode call, DataFlow::ObjectLiteralNode obj | + call.getAnArgument() = [lit.flow(), StringConcatenation::getRoot(lit.flow())] and obj.flowsTo(call.getAnArgument()) and forex(string name | name = lit.getAReferencedVariable() | exists(obj.getAPropertyWrite(name))) ) From d7ad625de3453e1e86da82b22394732c8737cf5e Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 12 Jun 2025 14:28:00 +0200 Subject: [PATCH 7/7] JS: restrict type tracking to strings of interest. --- .../TemplateSyntaxInStringLiteral.ql | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql b/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql index 4dd91b143dcd..f6824cfd9587 100644 --- a/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql +++ b/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql @@ -97,24 +97,26 @@ VarDecl getDeclIn(Variable v, Scope scope, string name, CandidateTopLevel tl) { /** * Tracks data flow from a string literal that may flow to a replace operation. */ -DataFlow::SourceNode trackString(CandidateStringLiteral lit, DataFlow::TypeTracker t) { - t.start() and result = lit.flow() +DataFlow::SourceNode trackStringWithTemplateSyntax( + CandidateStringLiteral lit, DataFlow::TypeTracker t +) { + t.start() and result = lit.flow() and exists(lit.getAReferencedVariable()) or - exists(DataFlow::TypeTracker t2 | result = trackString(lit, t2).track(t2, t)) + exists(DataFlow::TypeTracker t2 | result = trackStringWithTemplateSyntax(lit, t2).track(t2, t)) } /** * Gets a string literal that flows to a replace operation. */ -DataFlow::SourceNode trackString(CandidateStringLiteral lit) { - result = trackString(lit, DataFlow::TypeTracker::end()) +DataFlow::SourceNode trackStringWithTemplateSyntax(CandidateStringLiteral lit) { + result = trackStringWithTemplateSyntax(lit, DataFlow::TypeTracker::end()) } /** * Holds if the string literal flows to a replace method call. */ predicate hasReplaceMethodCall(CandidateStringLiteral lit) { - trackString(lit).getAMethodCall() instanceof StringReplaceCall + trackStringWithTemplateSyntax(lit).getAMethodCall() instanceof StringReplaceCall } from CandidateStringLiteral lit, Variable v, Scope s, string name, VarDecl decl