Merge pull request #2715 from erik-krogh/PrivateFields

Approved by asgerf

Author: semmle-qlci

javascript/extractor/src/com/semmle/jcorn/Parser.java

@@ -5,6 +5,7 @@
 
 import com.semmle.jcorn.Identifiers.Dialect;
 import com.semmle.jcorn.Options.AllowReserved;
+import com.semmle.jcorn.TokenType.Properties;
 import com.semmle.js.ast.ArrayExpression;
 import com.semmle.js.ast.ArrayPattern;
 import com.semmle.js.ast.ArrowFunctionExpression;
@@ -44,6 +45,7 @@
 import com.semmle.js.ast.IPattern;
 import com.semmle.js.ast.Identifier;
 import com.semmle.js.ast.IfStatement;
+import com.semmle.js.ast.FieldDefinition;
 import com.semmle.js.ast.ImportDeclaration;
 import com.semmle.js.ast.ImportDefaultSpecifier;
 import com.semmle.js.ast.ImportNamespaceSpecifier;
@@ -124,6 +126,7 @@ public class Parser {
   private boolean inModule;
   protected boolean inFunction;
   protected boolean inGenerator;
+  protected boolean inClass;
   protected boolean inAsync;
   protected boolean inTemplateElement;
   protected int pos;
@@ -240,8 +243,8 @@ public Parser(Options options, String input, int startPos) {
     // Used to signify the start of a potential arrow function
     this.potentialArrowAt = -1;
 
-    // Flags to track whether we are in a function, a generator, an async function.
-    this.inFunction = this.inGenerator = this.inAsync = false;
+    // Flags to track whether we are in a function, a generator, an async function, a class.
+    this.inFunction = this.inGenerator = this.inAsync = this.inClass = false;
     // Positions to delayed-check that yield/await does not exist in default parameters.
     this.yieldPos = this.awaitPos = 0;
     // Labels in scope.
@@ -651,6 +654,9 @@ protected Token getTokenFromCode(int code) {
       case 58:
         ++this.pos;
         return this.finishToken(TokenType.colon);
+      case 35: 
+        ++this.pos;
+        return this.finishToken(TokenType.pound);
       case 63:
         return this.readToken_question();
 
@@ -2191,6 +2197,7 @@ protected Expression processExprListItem(Expression e) {
   // identifiers.
   protected Identifier parseIdent(boolean liberal) {
     Position startLoc = this.startLoc;
+    boolean isPrivateField = liberal && this.eat(TokenType.pound);
     if (liberal && this.options.allowReserved() == AllowReserved.NEVER) liberal = false;
     String name = null;
     if (this.type == TokenType.name) {
@@ -2199,9 +2206,9 @@ protected Identifier parseIdent(boolean liberal) {
           && (this.options.ecmaVersion() >= 6
               || inputSubstring(this.start, this.end).indexOf("\\") == -1))
         this.raiseRecoverable(this.start, "The keyword '" + this.value + "' is reserved");
-      if (this.inGenerator && this.value.equals("yield"))
+      if (!isPrivateField && this.inGenerator && this.value.equals("yield"))
         this.raiseRecoverable(this.start, "Can not use 'yield' as identifier inside a generator");
-      if (this.inAsync && this.value.equals("await"))
+      if (!isPrivateField && this.inAsync && this.value.equals("await"))
         this.raiseRecoverable(
             this.start, "Can not use 'await' as identifier inside an async function");
       name = String.valueOf(this.value);
@@ -2213,6 +2220,12 @@ protected Identifier parseIdent(boolean liberal) {
       this.unexpected();
     }
     this.next();
+    if (isPrivateField) {
+      if (!this.inClass) {
+        this.raiseRecoverable(this.start, "Cannot use private fields outside a class");
+      }
+      name = "#" + name;
+    }
     Identifier node = new Identifier(new SourceLocation(startLoc), name);
     return this.finishNode(node);
   }
@@ -3127,6 +3140,8 @@ protected List<Expression> parseFunctionParams() {
   // Parse a class declaration or literal (depending on the
   // `isStatement` parameter).
   protected Node parseClass(Position startLoc, boolean isStatement) {
+    boolean oldInClass = this.inClass;
+    this.inClass = true;
     SourceLocation loc = new SourceLocation(startLoc);
     this.next();
     Identifier id = this.parseClassId(isStatement);
@@ -3145,6 +3160,8 @@ protected Node parseClass(Position startLoc, boolean isStatement) {
     Node node;
     if (isStatement) node = new ClassDeclaration(loc, id, superClass, classBody);
     else node = new ClassExpression(loc, id, superClass, classBody);
+
+    this.inClass = oldInClass;
     return this.finishNode(node);
   }
 
@@ -3221,6 +3238,9 @@ protected MemberDefinition<?> parseClassPropertyBody(
       if (pi.kind.equals("set") && node.getValue().hasRest())
         this.raiseRecoverable(params.get(params.size() - 1), "Setter cannot use rest params");
     }
+    if (pi.key instanceof Identifier && ((Identifier)pi.key).getName().startsWith("#")) {
+      raiseRecoverable(pi.key, "Only fields, not methods, can be declared private.");
+    }
     return node;
   }
 

javascript/extractor/src/com/semmle/jcorn/TokenType.java

@@ -85,6 +85,7 @@ public void updateContext(Parser parser, TokenType prevType) {
       dot = new TokenType(new Properties(".")),
       questiondot = new TokenType(new Properties("?.")),
       question = new TokenType(new Properties("?").beforeExpr()),
+      pound = new TokenType(kw("#")),
       arrow = new TokenType(new Properties("=>").beforeExpr()),
       template = new TokenType(new Properties("template")),
       invalidTemplate = new TokenType(new Properties("invalidTemplate")),

javascript/extractor/tests/shebang/output/trap/tst.html.trap

@@ -14,7 +14,7 @@ toplevels(#20001,1)
 locations_default(#20002,#10000,3,17,3,17)
 hasLocation(#20001,#20002)
 #20003=*
-jsParseErrors(#20003,#20001,"Error: Unexpected character '#' (U+0023)","#!/usr/bin/node
+jsParseErrors(#20003,#20001,"Error: Unexpected token","#!/usr/bin/node
 ")
 #20004=@"loc,{#10000},4,1,4,1"
 locations_default(#20004,#10000,4,1,4,1)

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

@@ -535,6 +535,13 @@ module DataFlow {
      */
     pragma[noinline]
     predicate accesses(Node base, string p) { getBase() = base and getPropertyName() = p }
+    
+    /**
+     * Holds if this data flow node reads or writes a private field in a class.
+     */ 
+    predicate isPrivateField() {
+      getPropertyName().charAt(0) = "#" and getPropertyNameExpr() instanceof Label
+    }
   }
 
   /**

javascript/ql/test/library-tests/Classes/ClassFlow.qll

@@ -0,0 +1,17 @@
+import javascript
+
+class Configuration extends DataFlow::Configuration {
+  Configuration() { this = "ClassDataFlowTestingConfig" }
+
+  override predicate isSource(DataFlow::Node source) {
+    source.getEnclosingExpr().(StringLiteral).getValue().toLowerCase() = "source"
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    any(DataFlow::CallNode call | call.getCalleeName() = "sink").getAnArgument() = sink
+  }
+}
+
+query predicate dataflow(DataFlow::Node pred, DataFlow::Node succ) {
+  any(Configuration c).hasFlow(pred, succ)
+}

javascript/ql/test/library-tests/Classes/PrivateField.qll

@@ -0,0 +1,9 @@
+import javascript
+
+query string getAccessModifier(DataFlow::PropRef ref, Expr prop) {
+  prop = ref.getPropertyNameExpr() and
+  if ref.isPrivateField() then
+    result = "Private"
+  else 
+    result = "Public"
+}

javascript/ql/test/library-tests/Classes/dataflow.js

@@ -0,0 +1,17 @@
+(function () {
+	var source = "source";
+	
+	class Foo {
+		#priv = source;
+		getPriv() {
+			return this.#priv;
+		}
+		
+		getFalsePrivate() {
+			return this["#priv"]; // not the same field as above. But we currently don't distinguish private and "public" fields.  
+		}
+	}
+	sink(new Foo().getPriv()); // NOT OK.
+	
+	sink(new Foo().getFalsePrivate()); // OK [FP: Is FP because we do nothing special about dataflow for private fields.]
+})();

javascript/ql/test/library-tests/Classes/privateFields.js

@@ -0,0 +1,27 @@
+class Foo {
+	#privDecl = 3;
+	#if = "if"; // "keywords" are ok.
+	reads() {
+		var foo = this.#privUse;
+		var bar = this["#publicComputed"]
+		var baz = this.#if;
+	}
+	
+	equals(o) {
+		return this.#privDecl === o.#privDecl;
+	}
+	
+	writes() {
+		this.#privDecl = 4;		
+		this["#public"] = 5;
+	}
+	
+	#privSecond; // is a PropNode, not a PropRef. Doesn't matter.
+	
+	["#publicField"] = 6;
+	
+	calls() {
+		this.#privDecl();
+		new this.#privDecl();
+	}
+}