JS: Only extract local vars in TemplateTopLevel

Angular template expressions cannot refer to global variables, any
unqualified identifier is a reference to a property provided by the
component.

We extract them as implicitly declared local variables which the
QL model can then connect with data flow steps.

javascript/extractor/src/com/semmle/js/extractor/ASTExtractor.java

@@ -704,7 +704,10 @@ public class ASTExtractor {
                     + locationManager.getStartLine()
                     + ","
                     + locationManager.getStartColumn());
-        scopeManager.enterScope(ScopeKind.module, moduleScopeKey, toplevelLabel);
+        Scope moduleScope = scopeManager.enterScope(ScopeKind.module, moduleScopeKey, toplevelLabel);
+        if (sourceType.hasNoGlobalScope()) {
+          scopeManager.setImplicitVariableScope(moduleScope);
+        }
         scopeManager.addVariables(
             sourceType.getPredefinedLocals(platform, locationManager.getSourceFileExtension()));
         trapwriter.addTuple("is_module", toplevelLabel);

javascript/extractor/src/com/semmle/js/extractor/ExtractorConfig.java

@@ -113,6 +113,14 @@ public class ExtractorConfig {
       return this != SCRIPT;
     }
 
+    /**
+     * Returns true if this source type cannot access the global scope directly, and undeclared
+     * variables are implicitly declared in its local scope. Implies {@link #hasLocalScope()}.
+     */
+    public boolean hasNoGlobalScope() {
+      return this == ANGULAR_TEMPLATE;
+    }
+
     /** Returns true if this source is implicitly in strict mode. */
     public boolean isStrictMode() {
       return this == MODULE;

javascript/extractor/src/com/semmle/js/extractor/HTMLExtractor.java

@@ -118,7 +118,7 @@ public class HTMLExtractor implements IExtractor {
                 }
               }
               extractSnippet(
-                  TopLevelKind.eventHandler,
+                  TopLevelKind.angularTemplate,
                   config.withSourceType(SourceType.ANGULAR_TEMPLATE),
                   scopeManager,
                   textualExtractor,

javascript/extractor/src/com/semmle/js/extractor/ScopeManager.java

@@ -102,11 +102,21 @@ public class ScopeManager {
   private final Scope toplevelScope;
   private final ECMAVersion ecmaVersion;
   private final Set<String> implicitGlobals = new LinkedHashSet<String>();
+  private Scope implicitVariableScope;
 
   public ScopeManager(TrapWriter trapWriter, ECMAVersion ecmaVersion) {
     this.trapWriter = trapWriter;
     this.toplevelScope = enterScope(ScopeKind.global, trapWriter.globalID("global_scope"), null);
     this.ecmaVersion = ecmaVersion;
+    this.implicitVariableScope = toplevelScope; 
+  }
+
+  /**
+   * Sets the scope in which to declare variables that are referenced without
+   * being declared. This defaults to the global scope.
+   */
+  public void setImplicitVariableScope(Scope implicitVariableScope) {
+    this.implicitVariableScope = implicitVariableScope;
   }
 
   /**
@@ -193,12 +203,12 @@ public class ScopeManager {
 
   /**
    * Get the label for a given variable in the current scope; if it cannot be found, add it to the
-   * global scope.
+   * implicit variable scope (usually the global scope). 
    */
   public Label getVarKey(String name) {
     Label lbl = curScope.lookupVariable(name);
     if (lbl == null) {
-      lbl = addVariable(name, toplevelScope);
+      lbl = addVariable(name, implicitVariableScope);
       implicitGlobals.add(name);
     }
     return lbl;

javascript/extractor/src/com/semmle/js/extractor/TopLevelKind.java

@@ -7,7 +7,8 @@ public enum TopLevelKind {
   script(0),
   inlineScript(1),
   eventHandler(2),
-  javascriptUrl(3);
+  javascriptUrl(3),
+  angularTemplate(4);
   
   private int value;
   

javascript/ql/src/semmle/javascript/AST.qll

@@ -300,7 +300,7 @@ class InlineScript extends @inline_script, Script { }
  * ```
  */
 class CodeInAttribute extends TopLevel {
-  CodeInAttribute() { this instanceof @event_handler or this instanceof @javascript_url }
+  CodeInAttribute() { this instanceof @event_handler or this instanceof @javascript_url or this instanceof @angular_template_toplevel }
 }
 
 /**

javascript/ql/src/semmle/javascript/dataflow/Sources.qll

@@ -309,7 +309,8 @@ module SourceNode {
         astNode instanceof ImportSpecifier or
         astNode instanceof ImportMetaExpr or
         astNode instanceof TaggedTemplateExpr or
-        astNode instanceof Angular2::PipeRefExpr
+        astNode instanceof Angular2::PipeRefExpr or
+        astNode instanceof Angular2::TemplateVarRefExpr
       )
       or
       DataFlow::parameterNode(this, _)

javascript/ql/src/semmle/javascript/dataflow/internal/VariableTypeInference.qll

@@ -307,6 +307,11 @@ class AnalyzedNegativeConditionGuard extends AnalyzedRefinement {
   }
 }
 
+/** Holds if `v` is a variable in an Angular template. */
+private predicate isAngularTemplateVariable(LocalVariable v) {
+  v = any(Angular2::TemplateTopLevel tl).getScope().getAVariable()
+}
+
 /**
  * Gets the abstract value representing the initial value of variable `v`.
  *
@@ -325,7 +330,10 @@ private AbstractValue getImplicitInitValue(LocalVariable v) {
       then
         // model hoisting
         result = TAbstractFunction(getAFunDecl(v))
-      else result = TAbstractUndefined()
+      else
+        if isAngularTemplateVariable(v)
+        then result = TIndefiniteAbstractValue("heap")
+        else result = TAbstractUndefined()
 }
 
 /**

javascript/ql/src/semmle/javascript/frameworks/Angular2.qll

@@ -241,6 +241,33 @@ module Angular2 {
     }
   }
 
+  /**
+   * A reference to a variable in a template expression, corresponding
+   * to a property on the component class.
+   */
+  class TemplateVarRefExpr extends Expr {
+    TemplateVarRefExpr() {
+      this = any(TemplateTopLevel tl).getScope().getAVariable().getAnAccess()
+    }
+  }
+
+  /** The top-level containing an Angular expression. */
+  class TemplateTopLevel extends TopLevel, @angular_template_toplevel {
+    /** Gets the expression in this top-level. */
+    Expr getExpression() {
+      result = getChildStmt(0).(ExprStmt).getExpr()
+    }
+
+    /** Gets the data flow node representing the initialization of the given variable in this scope. */
+    DataFlow::Node getVariableInit(string name) {
+      result = DataFlow::ssaDefinitionNode(SSA::implicitInit(getScope().getVariable(name)))
+    }
+
+    DataFlow::SourceNode getAVariableUse(string name) {
+      result = getScope().getVariable(name).getAnAccess().flow()
+    }
+  }
+
   /** The RHS of a `templateUrl` property, seen as a path expression. */
   private class TemplateUrlPath extends PathExpr {
     TemplateUrlPath() {
@@ -264,18 +291,8 @@ module Angular2 {
     attrib.getName().matches("*ng%")
   }
 
-  /**
-   * Gets a global variable access to `name` within the given attribute.
-   */
-  pragma[noinline]
-  private GlobalVarAccess getAGlobalVarAccessInAttribute(CodeInAttribute code, string name) {
-    exists(ComponentClass cls) and // do not materialize for non-Angular codebases
-    result.getTopLevel() = code and
-    result.getName() = name
-  }
-
   private DataFlow::Node getAttributeValueAsNode(HTML::Attribute attrib) {
-    result = attrib.getCodeInAttribute().getChildStmt(0).(ExprStmt).getExpr().flow()
+    result = attrib.getCodeInAttribute().(TemplateTopLevel).getExpression().flow()
   }
 
   /**
@@ -361,11 +378,7 @@ module Angular2 {
      * Gets an access to the variable `name` in the template body.
      */
     DataFlow::Node getATemplateVarAccess(string name) {
-      exists(HTML::Attribute attrib |
+      result = getATemplateElement().getAnAttribute().getCodeInAttribute().(TemplateTopLevel).getAVariableUse(name)
-        attrib = getATemplateElement().getAnAttribute() and
-        isAngularExpressionAttribute(attrib) and
-        result = getAGlobalVarAccessInAttribute(attrib.getCodeInAttribute(), name).flow()
-      )
     }
   }
 
@@ -450,11 +463,7 @@ module Angular2 {
 
     /** Gets a reference to the iterator variable. */
     DataFlow::Node getAnIteratorAccess() {
-      exists(HTML::Attribute attrib |
+      result = getAnElementInScope().getAnAttribute().getCodeInAttribute().(TemplateTopLevel).getAVariableUse(getIteratorName())
-        attrib = getAnElementInScope().getAnAttribute() and
-        isAngularExpressionAttribute(attrib) and
-        result = getAGlobalVarAccessInAttribute(attrib.getCodeInAttribute(), getIteratorName()).flow()
-      )
     }
   }
 
@@ -474,4 +483,13 @@ module Angular2 {
       succ = attrib.getAnIteratorAccess()
     }
   }
+
+  private class AnyCastStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
+    AnyCastStep() { this = any(TemplateTopLevel tl).getAVariableUse("$any").getACall() }
+
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      pred = getArgument(0) and
+      succ = this
+    }
+  }
 }

javascript/ql/src/semmlecode.javascript.dbscheme

@@ -124,7 +124,8 @@ case @toplevel.kind of
    0 = @script
 |  1 = @inline_script
 |  2 = @event_handler
-|  3 = @javascript_url;
+|  3 = @javascript_url
+|  4 = @angular_template_toplevel;
 
 is_module (int tl: @toplevel ref);
 is_nodejs (int tl: @toplevel ref);

javascript/ql/test/library-tests/frameworks/Angular2/source.component.html

@@ -4,6 +4,7 @@
     [sink3]="taint | unknownPipe:'safe'"
     [sink4]="taint | testPipe:'safe'"
     [sink5]="42 | testPipe:taint"
+    (someEvent)="methodOnComponent(taint)"
 ></sink-component>
 
 <div *ngFor="let element of taintedArray">

javascript/ql/test/library-tests/frameworks/Angular2/source.component.ts

@@ -1,4 +1,5 @@
 import { Component } from "@angular/core";
+import { DomSanitizer } from '@angular/platform-browser';
 
 @Component({
     selector: "source-component",
@@ -9,9 +10,13 @@ export class Source {
     taintedArray: string[];
     safeArray: string[];
 
-    constructor() {
+    constructor(private sanitizer: DomSanitizer) {
         this.taint = source();
         this.taintedArray = [...source()];
         this.safeArray = ['a', 'b'];
     }
+
+    methodOnComponent(x) {
+        this.sanitizer.bypassSecurityTrustHtml(x);
+    }
 }

javascript/ql/test/library-tests/frameworks/Angular2/test.expected

@@ -23,8 +23,9 @@ pipeClassRef
 | TestPipe.ts:4:8:9:1 | class T ... ;\\n  }\\n} | source.component.html:6:19:6:26 | testPipe |
 taintFlow
 | inline.component.ts:15:22:15:29 | source() | sink.component.ts:26:48:26:57 | this.sink7 |
-| source.component.ts:13:22:13:29 | source() | sink.component.ts:20:48:20:57 | this.sink1 |
+| source.component.ts:14:22:14:29 | source() | sink.component.ts:20:48:20:57 | this.sink1 |
-| source.component.ts:13:22:13:29 | source() | sink.component.ts:23:48:23:57 | this.sink4 |
+| source.component.ts:14:22:14:29 | source() | sink.component.ts:23:48:23:57 | this.sink4 |
-| source.component.ts:13:22:13:29 | source() | sink.component.ts:24:48:24:57 | this.sink5 |
+| source.component.ts:14:22:14:29 | source() | sink.component.ts:24:48:24:57 | this.sink5 |
-| source.component.ts:13:22:13:29 | source() | sink.component.ts:25:48:25:57 | this.sink6 |
+| source.component.ts:14:22:14:29 | source() | sink.component.ts:25:48:25:57 | this.sink6 |
-| source.component.ts:14:33:14:40 | source() | sink.component.ts:20:48:20:57 | this.sink1 |
+| source.component.ts:14:22:14:29 | source() | source.component.ts:20:48:20:48 | x |
+| source.component.ts:15:33:15:40 | source() | sink.component.ts:20:48:20:57 | this.sink1 |