Merge pull request github#5739 from RasmusWL/share-sensitive-data-modeling

Python/JS: Share sensitive data modeling

Author: yoff

config/identical-files.json

@@ -440,5 +440,9 @@
   "CryptoAlgorithms Python/JS": [
     "javascript/ql/src/semmle/javascript/security/CryptoAlgorithms.qll",
     "python/ql/src/semmle/crypto/Crypto.qll"
+  ],
+  "SensitiveDataHeuristics Python/JS": [
+    "javascript/ql/src/semmle/javascript/security/internal/SensitiveDataHeuristics.qll",
+    "python/ql/src/semmle/python/security/internal/SensitiveDataHeuristics.qll"
   ]
-}
+}

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

@@ -10,67 +10,7 @@
  */
 
 import javascript
-
-/**
- * Provides heuristics for identifying names related to sensitive information.
- *
- * INTERNAL: Do not use directly.
- */
-module HeuristicNames {
-  /**
-   * 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 may indicate the presence of
-   * user names or other account information.
-   */
-  string maybeAccountInfo() {
-    result = "(?is).*acc(ou)?nt.*" or
-    result = "(?is).*(puid|username|userid).*"
-  }
-
-  /**
-   * 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).*(auth(entication|ori[sz]ation)?)key.*"
-  }
-
-  /**
-   * 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 may indicate the presence of data
-   * that is hashed or encrypted, and hence rendered non-sensitive, or contains special characters
-   * suggesting nouns within the string do not represent the meaning of the whole string (e.g. a URL or a SQL query).
-   */
-  string notSensitive() {
-    result = "(?is).*([^\\w$.-]|redact|censor|obfuscate|hash|md5|sha|((?<!un)(en))?(crypt|code)).*"
-  }
-}
-
+import semmle.javascript.security.internal.SensitiveDataHeuristics
 private import HeuristicNames
 
 /** An expression that might contain sensitive data. */
@@ -79,56 +19,44 @@ abstract class SensitiveExpr extends Expr {
   abstract string describe();
 
   /** Gets a classification of the kind of sensitive data this expression might contain. */
-  abstract SensitiveExpr::Classification getClassification();
+  abstract SensitiveDataClassification 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" }
-  }
+/** DEPRECATED: Use `SensitiveDataClassification` and helpers instead. */
+deprecated module SensitiveExpr {
+  /** DEPRECATED: Use `SensitiveDataClassification` instead. */
+  deprecated class Classification = SensitiveDataClassification;
 
-  /** Gets the classification for secret or trusted data. */
-  Classification secret() { result = "secret" }
+  /** DEPRECATED: Use `SensitiveDataClassification::secret` instead. */
+  deprecated predicate secret = SensitiveDataClassification::secret/0;
 
-  /** Gets the classification for user names or other account information. */
-  Classification id() { result = "id" }
+  /** DEPRECATED: Use `SensitiveDataClassification::id` instead. */
+  deprecated predicate id = SensitiveDataClassification::id/0;
 
-  /** Gets the classification for passwords or authorization keys. */
-  Classification password() { result = "password" }
+  /** DEPRECATED: Use `SensitiveDataClassification::password` instead. */
+  deprecated predicate password = SensitiveDataClassification::password/0;
 
-  /** Gets the classification for certificates. */
-  Classification certificate() { result = "certificate" }
+  /** DEPRECATED: Use `SensitiveDataClassification::certificate` instead. */
+  deprecated predicate certificate = SensitiveDataClassification::certificate/0;
 }
 
 /** A function call that might produce sensitive data. */
 class SensitiveCall extends SensitiveExpr, InvokeExpr {
-  SensitiveExpr::Classification classification;
+  SensitiveDataClassification classification;
 
   SensitiveCall() {
     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(classification)) and
-      not s.regexpMatch(notSensitive())
+      nameIndicatesSensitiveData(s, classification)
     )
   }
 
   override string describe() { result = "a call to " + getCalleeName() }
 
-  override SensitiveExpr::Classification getClassification() { result = classification }
+  override SensitiveDataClassification getClassification() { result = classification }
 }
 
 /** An access to a variable or property that might contain sensitive data. */
@@ -152,13 +80,10 @@ 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;
+  SensitiveDataClassification classification;
 
   BasicSensitiveWrite() {
-    exists(string name |
-      name.regexpMatch(maybeSensitive(classification)) and
-      not name.regexpMatch(notSensitive())
-    |
+    exists(string name | nameIndicatesSensitiveData(name, classification) |
       exists(DataFlow::PropWrite pwn |
         pwn.getPropertyName() = name and
         pwn.getRhs() = this
@@ -173,18 +98,16 @@ private class BasicSensitiveWrite extends SensitiveWrite {
   }
 
   /** Gets a classification of the kind of sensitive data the write might handle. */
-  SensitiveExpr::Classification getClassification() { result = classification }
+  SensitiveDataClassification getClassification() { result = classification }
 }
 
 /** An access to a variable or property that might contain sensitive data. */
 private class BasicSensitiveVariableAccess extends SensitiveVariableAccess {
-  SensitiveExpr::Classification classification;
+  SensitiveDataClassification classification;
 
-  BasicSensitiveVariableAccess() {
-    name.regexpMatch(maybeSensitive(classification)) and not name.regexpMatch(notSensitive())
-  }
+  BasicSensitiveVariableAccess() { nameIndicatesSensitiveData(name, classification) }
 
-  override SensitiveExpr::Classification getClassification() { result = classification }
+  override SensitiveDataClassification getClassification() { result = classification }
 }
 
 /** A function name that suggests it may be sensitive. */
@@ -199,16 +122,16 @@ abstract class SensitiveFunctionName extends string {
 /** A function name that suggests it may produce sensitive data. */
 abstract class SensitiveDataFunctionName extends SensitiveFunctionName {
   /** Gets a classification of the kind of sensitive data this function may produce. */
-  abstract SensitiveExpr::Classification getClassification();
+  abstract SensitiveDataClassification getClassification();
 }
 
 /** A method that might return sensitive data, based on the name. */
 class CredentialsFunctionName extends SensitiveDataFunctionName {
-  SensitiveExpr::Classification classification;
+  SensitiveDataClassification classification;
 
-  CredentialsFunctionName() { this.regexpMatch(maybeSensitive(classification)) }
+  CredentialsFunctionName() { nameIndicatesSensitiveData(this, classification) }
 
-  override SensitiveExpr::Classification getClassification() { result = classification }
+  override SensitiveDataClassification getClassification() { result = classification }
 }
 
 /**
@@ -240,11 +163,13 @@ class ProtectCall extends DataFlow::CallNode {
 
 /** An expression that might contain a clear-text password. */
 class CleartextPasswordExpr extends SensitiveExpr {
-  CleartextPasswordExpr() { this.(SensitiveExpr).getClassification() = SensitiveExpr::password() }
+  CleartextPasswordExpr() {
+    this.(SensitiveExpr).getClassification() = SensitiveDataClassification::password()
+  }
 
   override string describe() { none() }
 
-  override SensitiveExpr::Classification getClassification() { none() }
+  override SensitiveDataClassification getClassification() { none() }
 }
 
 /**

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

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

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

@@ -35,7 +35,7 @@ module CleartextStorage {
 
     SensitiveExprSource() {
       // storing user names or account names in plaintext isn't usually a problem
-      astNode.getClassification() != SensitiveExpr::id()
+      astNode.getClassification() != SensitiveDataClassification::id()
     }
 
     override string describe() { result = astNode.describe() }

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

@@ -34,7 +34,9 @@ module InsufficientPasswordHash {
   class CleartextPasswordSource extends Source, DataFlow::ValueNode {
     override SensitiveExpr astNode;
 
-    CleartextPasswordSource() { astNode.getClassification() = SensitiveExpr::password() }
+    CleartextPasswordSource() {
+      astNode.getClassification() = SensitiveDataClassification::password()
+    }
 
     override string describe() { result = astNode.describe() }
   }