Merge pull request #4507 from erik-krogh/template

Approved by asgerf

Author: codeql-ci

javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql

@@ -62,8 +62,8 @@ class CandidateStringLiteral extends StringLiteral {
 }
 
 /**
- * Holds if `obj` has a property for each template variable in `lit` and they occur as arguments
- * to the same call.
+ * Holds if there exists an object that has a property for each template variable in `lit` and
+ * they occur as arguments to the same call.
  *
  * This recognises a typical pattern in which template arguments are passed along with a string,
  * for example:
@@ -73,14 +73,14 @@ class CandidateStringLiteral extends StringLiteral {
  *        { url: url, name: name } );
  * ```
  */
-predicate providesTemplateVariablesFor(ObjectExpr obj, CandidateStringLiteral lit) {
-  exists(CallExpr call | call.getAnArgument() = obj and call.getAnArgument() = lit) and
-  forex(string name | lit.getAReferencedVariable() = name | hasProperty(obj, name))
+predicate hasObjectProvidingTemplateVariables(CandidateStringLiteral lit) {
+  exists(DataFlow::CallNode call, DataFlow::ObjectLiteralNode obj |
+    call.getAnArgument().getALocalSource() = obj and
+    call.getAnArgument().asExpr() = lit and
+    forex(string name | name = lit.getAReferencedVariable() | exists(obj.getAPropertyWrite(name)))
+  )
 }
 
-/** Holds if `object` has a property with the given `name`. */
-predicate hasProperty(ObjectExpr object, string name) { name = object.getAProperty().getName() }
-
 /**
  * Gets a declaration of variable `v` in `tl`, where `v` has the given `name` and
  * belongs to `scope`.
@@ -97,7 +97,7 @@ where
   decl = getDeclIn(v, s, name, lit.getTopLevel()) and
   lit.getAReferencedVariable() = name and
   lit.isInScope(s) and
-  not exists(ObjectExpr obj | providesTemplateVariablesFor(obj, lit)) and
+  not hasObjectProvidingTemplateVariables(lit) and
   not lit.getStringValue() = "${" + name + "}"
 select lit, "This string is not a template literal, but appears to reference the variable $@.",
   decl, v.getName()

javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.expected

@@ -2,3 +2,4 @@
 | TemplateSyntaxInStringLiteral.js:17:15:17:40 | '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: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 |

javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.js

@@ -28,3 +28,16 @@ function baz(x){
     log.error("${x} ");
     log.error("${y} ");
 }
+
+
+function foo1() {
+    const aTemplateLitInScope = `Connecting to ${id}`;
+    const name = 2;
+    const date = 3;
+    const foobar = 4;
+
+    const data = {name: name, date: date};
+    writer.emit("Name: ${name}, Date: ${date}.", data); // OK 
+
+    writer.emit("Name: ${name}, Date: ${date}, ${foobar}", data); // NOT OK - `foobar` is not in `data`.     
+}