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..f6824cfd9587 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 @@ -74,8 +77,8 @@ 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 + call.getAnArgument() = [lit.flow(), StringConcatenation::getRoot(lit.flow())] and + obj.flowsTo(call.getAnArgument()) and forex(string name | name = lit.getAReferencedVariable() | exists(obj.getAPropertyWrite(name))) ) } @@ -91,12 +94,38 @@ 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 trackStringWithTemplateSyntax( + CandidateStringLiteral lit, DataFlow::TypeTracker t +) { + t.start() and result = lit.flow() and exists(lit.getAReferencedVariable()) + or + exists(DataFlow::TypeTracker t2 | result = trackStringWithTemplateSyntax(lit, t2).track(t2, t)) +} + +/** + * Gets a string literal that flows to a replace operation. + */ +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) { + trackStringWithTemplateSyntax(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/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. diff --git a/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.js b/javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.js index d21a662dc5e8..9e0993278c16 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}", { + 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}"; + let result1 = replacer(str, name); + console.log(result1); + + str = "Name: ${name} and again: ${name}"; + let result2 = replacerAll(str, name); + console.log(result2); +}