JavaScript: Clean up classification of sensitive strings.

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

@@ -17,31 +17,44 @@ import javascript
  * INTERNAL: Do not use directly.
  */
 module HeuristicNames {
-  /** Gets a regular expression that identifies strings that look like they represent secret data that are not passwords. */
-  string suspiciousNonPassword() { result = "(?is).*(secret|account|accnt|(?<!un)trusted).*" }
-
-  /** Gets a regular expression that identifies strings that look like they represent secret data that are passwords. */
-  string suspiciousPassword() { result = "(?is).*(password|passwd).*" }
-
-  /** Gets a regular expression that identifies strings that look like they represent secret data. */
-  string suspicious() { result = suspiciousPassword() or result = suspiciousNonPassword() }
+  /**
+   * Gets a regular expression that identifies strings that look like they represent 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 data that is
-   * hashed or encrypted.
+   * Gets a regular expression that identifies strings that look like they represent user names or other
+   * account information.
    */
-  string nonSuspicious() {
-    result = "(?is).*(redact|censor|obfuscate|hash|md5|sha|((?<!un)(en))?(crypt|code)).*"
+  string maybeAccountInfo() {
+    result = "(?is).*acc(ou)?nt.*" or
+    result = "(?is).*(puid|username|userid).*"
   }
 
   /**
-   * Gets a regular expression that identifies names that look like they represent credential information.
+   * Gets a regular expression that identifies strings that look like they represent a password,
+   * a certificate, an authorization key, or similar.
    */
-  string suspiciousCredentials() {
-    result = "(?i).*pass(wd|word|code|phrase)(?!.*question).*" or
-    result = "(?i).*(puid|username|userid).*" or
-    result = "(?i).*(cert)(?!.*(format|name)).*" or
-    result = "(?i).*(auth(entication|ori[sz]ation)?)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 look like they represent 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)).*"
   }
 }
 private import HeuristicNames
@@ -60,8 +73,8 @@ class SensitiveCall extends SensitiveExpr, InvokeExpr {
     // 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(suspicious()) and
-      not s.regexpMatch(nonSuspicious())
+      s.regexpMatch(maybeSensitive()) and
+      not s.regexpMatch(notSensitive())
     )
   }
 
@@ -91,8 +104,8 @@ abstract class SensitiveWrite extends DataFlow::Node { }
 private class BasicSensitiveWrite extends SensitiveWrite {
   BasicSensitiveWrite() {
     exists(string name |
-      name.regexpMatch(suspicious()) and
-      not name.regexpMatch(nonSuspicious())
+      name.regexpMatch(maybeSensitive()) and
+      not name.regexpMatch(notSensitive())
     |
       exists(DataFlow::PropWrite pwn |
         pwn.getPropertyName() = name and
@@ -115,7 +128,7 @@ private class BasicSensitiveWrite extends SensitiveWrite {
 /** An access to a variable or property that might contain sensitive data. */
 private class BasicSensitiveVariableAccess extends SensitiveVariableAccess {
   BasicSensitiveVariableAccess() {
-    name.regexpMatch(suspicious()) and not name.regexpMatch(nonSuspicious())
+    name.regexpMatch(maybeSensitive()) and not name.regexpMatch(notSensitive())
   }
 }
 
@@ -133,7 +146,7 @@ abstract class SensitiveDataFunctionName extends SensitiveFunctionName { }
 
 /** A method that might return sensitive data, based on the name. */
 class CredentialsFunctionName extends SensitiveDataFunctionName {
-  CredentialsFunctionName() { this.regexpMatch(suspicious()) }
+  CredentialsFunctionName() { this.regexpMatch(maybeSensitive()) }
 }
 
 /**
@@ -169,8 +182,8 @@ class ProtectCall extends DataFlow::CallNode {
 private module CleartextPasswords {
   bindingset[name]
   private predicate isCleartextPasswordIndicator(string name) {
-    name.regexpMatch(suspiciousPassword()) and
-    not name.regexpMatch(nonSuspicious())
+    name.regexpMatch(maybePassword()) and
+    not name.regexpMatch(notSensitive())
   }
 
   /** An expression that might contain a cleartext password. */

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

@@ -70,7 +70,7 @@ module CleartextLogging {
    */
   private class NameGuidedNonCleartextPassword extends NonCleartextPassword {
     NameGuidedNonCleartextPassword() {
-      exists(string name | name.regexpMatch(nonSuspicious()) |
+      exists(string name | name.regexpMatch(notSensitive()) |
         this.asExpr().(VarAccess).getName() = name
         or
         this.(DataFlow::PropRead).getPropertyName() = name
@@ -111,7 +111,7 @@ module CleartextLogging {
    * A call that might obfuscate a password, for example through hashing.
    */
   private class ObfuscatorCall extends Barrier, DataFlow::InvokeNode {
-    ObfuscatorCall() { getCalleeName().regexpMatch(nonSuspicious()) }
+    ObfuscatorCall() { getCalleeName().regexpMatch(notSensitive()) }
   }
 
   /**
@@ -129,8 +129,8 @@ module CleartextLogging {
 
     ObjectPasswordPropertySource() {
       exists(DataFlow::PropWrite write |
-        name.regexpMatch(suspiciousPassword()) and
-        not name.regexpMatch(nonSuspicious()) and
+        name.regexpMatch(maybePassword()) and
+        not name.regexpMatch(notSensitive()) and
         write = this.(DataFlow::SourceNode).getAPropertyWrite(name) and
         // avoid safe values assigned to presumably unsafe names
         not write.getRhs() instanceof NonCleartextPassword
@@ -147,7 +147,7 @@ module CleartextLogging {
     ReadPasswordSource() {
       // avoid safe values assigned to presumably unsafe names
       not this instanceof NonCleartextPassword and
-      name.regexpMatch(suspiciousPassword()) and
+      name.regexpMatch(maybePassword()) and
       (
         this.asExpr().(VarAccess).getName() = name
         or

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

@@ -4,7 +4,7 @@
  */
 
 import javascript
-private import semmle.javascript.security.SensitiveActions
+private import semmle.javascript.security.SensitiveActions::HeuristicNames
 
 module PostMessageStar {
   /**
@@ -70,7 +70,7 @@ module PostMessageStar {
       )
       or
       // `toString` or `JSON.toString` on a partially tainted object gives a tainted value
-      exists (DataFlow::InvokeNode toString | toString = trg |
+      exists(DataFlow::InvokeNode toString | toString = trg |
         toString.(DataFlow::MethodCallNode).calls(src, "toString")
         or
         toString = DataFlow::globalVarRef("JSON").getAMemberCall("stringify") and
@@ -92,23 +92,6 @@ module PostMessageStar {
    */
   class SensitiveExprSource extends Source, DataFlow::ValueNode { override SensitiveExpr astNode; }
 
-  /**
-   * A variable/property access or function call whose name suggests that it may contain credentials,
-   * viewed as a data flow source for cross-window communication with unrestricted origin.
-   */
-  class CredentialsSource extends Source {
-    CredentialsSource() {
-      exists(string name |
-        name = this.(DataFlow::InvokeNode).getCalleeName() or
-        name = this.(DataFlow::PropRead).getPropertyName() or
-        name = this.asExpr().(VarUse).getVariable().getName()
-      |
-        name.regexpMatch(HeuristicNames::suspiciousCredentials()) and
-        not name.regexpMatch(HeuristicNames::nonSuspicious())
-      )
-    }
-  }
-
   /** A call to any function whose name suggests that it encodes or encrypts its arguments. */
   class ProtectSanitizer extends Sanitizer { ProtectSanitizer() { this instanceof ProtectCall } }