Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)))
)
}
Expand All @@ -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()
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: queryMetadata
---
* Added `reliability` and `language-features` tags to the `js/template-syntax-in-string-literal` query.
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
}
}

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);
}