JavaScript: Add classification of sensitive expressions.

We now classify sensitive expressions into four categories (secret, id, password, certificate). This allows queries more fine-grained control over what kinds of sensitive data they want to deal with: for clear-text storage, for instance, user ids aren't so much of a problem.

javascript/ql/src/semmle/javascript/security/SensitiveActions.qll

@@ -18,14 +18,14 @@ import javascript
  */
 module HeuristicNames {
   /**
-   * Gets a regular expression that identifies strings that look like they represent secret
+   * Gets a regular expression that identifies strings that may indicate the presence of secret
    * or trusted data.
    */
   string maybeSecret() { result = "(?is).*((?<!is)secret|(?<!un|is)trusted).*" }
 
   /**
-   * Gets a regular expression that identifies strings that look like they represent user names or other
-   * account information.
+   * Gets a regular expression that identifies strings that may indicate the presence of
+   * user names or other account information.
    */
   string maybeAccountInfo() {
     result = "(?is).*acc(ou)?nt.*" or
@@ -33,25 +33,37 @@ module HeuristicNames {
   }
 
   /**
-   * Gets a regular expression that identifies strings that look like they represent a password,
-   * a certificate, an authorization key, or similar.
+   * Gets a regular expression that identifies strings that may indicate the presence of
+   * a password or an authorization key.
    */
   string maybePassword() {
     result = "(?is).*pass(wd|word|code|phrase)(?!.*question).*" or
-    result = "(?is).*(cert)(?!.*(format|name)).*" or
     result = "(?is).*(auth(entication|ori[sz]ation)?)key.*"
   }
 
-  /** Gets a regular expression that identifies strings that look like they represent secret data. */
-  string maybeSensitive() {
-    result = maybeSecret() or
-    result = maybeAccountInfo() or
-    result = maybePassword()
+  /**
+   * Gets a regular expression that identifies strings that may indicate the presence of
+   * a certificate.
+   */
+  string maybeCertificate() { result = "(?is).*(cert)(?!.*(format|name)).*" }
+
+  /**
+   * Gets a regular expression that identifies strings that may indicate the presence
+   * of sensitive data, with `classification` describing the kind of sensitive data involved.
+   */
+  string maybeSensitive(SensitiveExpr::Classification classification) {
+    result = maybeSecret() and classification = SensitiveExpr::secret()
+    or
+    result = maybeAccountInfo() and classification = SensitiveExpr::id()
+    or
+    result = maybePassword() and classification = SensitiveExpr::password()
+    or
+    result = maybeCertificate() and classification = SensitiveExpr::certificate()
   }
 
   /**
-   * Gets a regular expression that identifies strings that look like they represent data that is
-   * hashed or encrypted, and hence rendered non-sensitive.
+   * Gets a regular expression that identifies strings that may indicate the presence of data
+   * that is hashed or encrypted, and hence rendered non-sensitive.
    */
   string notSensitive() {
     result = "(?is).*(redact|censor|obfuscate|hash|md5|sha|((?<!un)(en))?(crypt|code)).*"
@@ -63,22 +75,60 @@ private import HeuristicNames
 abstract class SensitiveExpr extends Expr {
   /** Gets a human-readable description of this expression for use in alert messages. */
   abstract string describe();
+
+  /** Gets a classification of the kind of sensitive data this expression might contain. */
+  abstract SensitiveExpr::Classification getClassification();
+}
+
+module SensitiveExpr {
+  /**
+   * A classification of different kinds of sensitive data:
+   *
+   *   - secret: generic secret or trusted data;
+   *   - id: a user name or other account information;
+   *   - password: a password or authorization key;
+   *   - certificate: a certificate.
+   *
+   * While classifications are represented as strings, this should not be relied upon.
+   * Instead, use the predicates below to work with classifications.
+   */
+  class Classification extends string {
+    Classification() {
+      this = "secret" or this = "id" or this = "password" or this = "certificate"
+    }
+  }
+
+  /** Gets the classification for secret or trusted data. */
+  Classification secret() { result = "secret" }
+
+  /** Gets the classification for user names or other account information. */
+  Classification id() { result = "id" }
+
+  /** Gets the classification for passwords or authorization keys. */
+  Classification password() { result = "password" }
+
+  /** Gets the classification for certificates. */
+  Classification certificate() { result = "certificate" }
 }
 
 /** A function call that might produce sensitive data. */
 class SensitiveCall extends SensitiveExpr, InvokeExpr {
+  SensitiveExpr::Classification classification;
+
   SensitiveCall() {
-    this.getCalleeName() instanceof SensitiveDataFunctionName
+    classification = this.getCalleeName().(SensitiveDataFunctionName).getClassification()
     or
     // This is particularly to pick up methods with an argument like "password", which
     // may indicate a lookup.
     exists(string s | this.getAnArgument().mayHaveStringValue(s) |
-      s.regexpMatch(maybeSensitive()) and
+      s.regexpMatch(maybeSensitive(classification)) and
       not s.regexpMatch(notSensitive())
     )
   }
 
   override string describe() { result = "a call to " + getCalleeName() }
+
+  override SensitiveExpr::Classification getClassification() { result = classification }
 }
 
 /** An access to a variable or property that might contain sensitive data. */
@@ -98,13 +148,16 @@ abstract class SensitiveVariableAccess extends SensitiveExpr {
 }
 
 /** A write to a location that might contain sensitive data. */
-abstract class SensitiveWrite extends DataFlow::Node { }
+abstract class SensitiveWrite extends DataFlow::Node {
+}
 
 /** A write to a variable or property that might contain sensitive data. */
 private class BasicSensitiveWrite extends SensitiveWrite {
+  SensitiveExpr::Classification classification;
+
   BasicSensitiveWrite() {
     exists(string name |
-      name.regexpMatch(maybeSensitive()) and
+      name.regexpMatch(maybeSensitive(classification)) and
       not name.regexpMatch(notSensitive())
     |
       exists(DataFlow::PropWrite pwn |
@@ -123,13 +176,20 @@ private class BasicSensitiveWrite extends SensitiveWrite {
       )
     )
   }
+
+  /** Gets a classification of the kind of sensitive data the write might handle. */
+  SensitiveExpr::Classification getClassification() { result = classification }
 }
 
 /** An access to a variable or property that might contain sensitive data. */
 private class BasicSensitiveVariableAccess extends SensitiveVariableAccess {
+  SensitiveExpr::Classification classification;
+
   BasicSensitiveVariableAccess() {
-    name.regexpMatch(maybeSensitive()) and not name.regexpMatch(notSensitive())
+    name.regexpMatch(maybeSensitive(classification)) and not name.regexpMatch(notSensitive())
   }
+
+  override SensitiveExpr::Classification getClassification() { result = classification }
 }
 
 /** A function name that suggests it may be sensitive. */
@@ -142,11 +202,18 @@ abstract class SensitiveFunctionName extends string {
 }
 
 /** A function name that suggests it may produce sensitive data. */
-abstract class SensitiveDataFunctionName extends SensitiveFunctionName { }
+abstract class SensitiveDataFunctionName extends SensitiveFunctionName {
+  /** Gets a classification of the kind of sensitive data this function may produce. */
+  abstract SensitiveExpr::Classification getClassification();
+}
 
 /** A method that might return sensitive data, based on the name. */
 class CredentialsFunctionName extends SensitiveDataFunctionName {
-  CredentialsFunctionName() { this.regexpMatch(maybeSensitive()) }
+  SensitiveExpr::Classification classification;
+
+  CredentialsFunctionName() { this.regexpMatch(maybeSensitive(classification)) }
+
+  override SensitiveExpr::Classification getClassification() { result = classification }
 }
 
 /**
@@ -176,41 +243,10 @@ class ProtectCall extends DataFlow::CallNode {
   }
 }
 
-/**
- * Classes for expressions containing cleartext passwords.
- */
-private module CleartextPasswords {
-  bindingset[name]
-  private predicate isCleartextPasswordIndicator(string name) {
-    name.regexpMatch(maybePassword()) and
-    not name.regexpMatch(notSensitive())
-  }
+/** An expression that might contain a clear-text password. */
+class CleartextPasswordExpr extends SensitiveExpr {
+  CleartextPasswordExpr() { this.(SensitiveExpr).getClassification() = SensitiveExpr::password() }
 
-  /** An expression that might contain a cleartext password. */
-  abstract class CleartextPasswordExpr extends SensitiveExpr { }
-
-  /** A function name that suggests it may produce a cleartext password. */
-  private class CleartextPasswordDataFunctionName extends SensitiveDataFunctionName {
-    CleartextPasswordDataFunctionName() { isCleartextPasswordIndicator(this) }
-  }
-
-  /** A call that might return a cleartext password. */
-  private class CleartextPasswordCallExpr extends CleartextPasswordExpr, SensitiveCall {
-    CleartextPasswordCallExpr() {
-      this.getCalleeName() instanceof CleartextPasswordDataFunctionName
-      or
-      // This is particularly to pick up methods with an argument like "password", which
-      // may indicate a lookup.
-      exists(string s |
-        this.getAnArgument().mayHaveStringValue(s) and
-        isCleartextPasswordIndicator(s)
-      )
-    }
-  }
-
-  /** An access to a variable or property that might contain a cleartext password. */
-  private class CleartextPasswordLookupExpr extends CleartextPasswordExpr, SensitiveVariableAccess {
-    CleartextPasswordLookupExpr() { isCleartextPasswordIndicator(name) }
-  }
+  override string describe() { none() }
+  override SensitiveExpr::Classification getClassification() { none() }
 }
-import CleartextPasswords

javascript/ql/src/semmle/javascript/security/dataflow/CleartextStorage.qll

@@ -50,6 +50,11 @@ module CleartextStorage {
   class SensitiveExprSource extends Source, DataFlow::ValueNode {
     override SensitiveExpr astNode;
 
+    SensitiveExprSource() {
+      // storing user names or account names in plaintext isn't usually a problem
+      astNode.getClassification() != SensitiveExpr::id()
+    }
+
     override string describe() { result = astNode.describe() }
   }
 

javascript/ql/src/semmle/javascript/security/dataflow/InsufficientPasswordHash.qll

@@ -52,7 +52,9 @@ module InsufficientPasswordHash {
    * with insufficient computational effort.
    */
   class CleartextPasswordSource extends Source, DataFlow::ValueNode {
-    override CleartextPasswordExpr astNode;
+    override SensitiveExpr astNode;
+
+    CleartextPasswordSource() { astNode.getClassification() = SensitiveExpr::password() }
 
     override string describe() { result = astNode.describe() }
   }