Java: Refactor Cleartext Storage queries

java/ql/lib/semmle/code/java/security/CleartextStorageClassQuery.qll

@@ -0,0 +1,105 @@
+/** Provides classes and predicates to reason about cleartext storage in serializable classes. */
+
+import java
+import semmle.code.java.frameworks.JAXB
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.dataflow.DataFlow2
+import semmle.code.java.security.CleartextStorageQuery
+import semmle.code.java.security.CleartextStoragePropertiesQuery
+
+private class ClassCleartextStorageSink extends CleartextStorageSink {
+  ClassCleartextStorageSink() { this.asExpr() = getInstanceInput(_, _) }
+}
+
+/** The instantiation of a storable class, which can be stored to disk. */
+abstract class ClassStore extends Storable, ClassInstanceExpr {
+  /** Gets an input, for example `input` in `instance.password = input`. */
+  override Expr getAnInput() {
+    exists(ClassStoreFlowConfig conf, DataFlow::Node instance |
+      conf.hasFlow(DataFlow::exprNode(this), instance) and
+      result = getInstanceInput(instance, this.getConstructor().getDeclaringType())
+    )
+  }
+}
+
+/**
+ * The instantiation of a serializable class, which can be stored to disk.
+ *
+ * Only includes tainted instances where data from a `SensitiveSource` may flow
+ * to an input of the `Serializable`.
+ */
+private class Serializable extends ClassStore {
+  Serializable() {
+    this.getConstructor().getDeclaringType().getASupertype*() instanceof TypeSerializable and
+    // `Properties` are `Serializable`, but handled elsewhere.
+    not this instanceof Properties and
+    // restrict attention to tainted instances
+    exists(SensitiveSource data |
+      data.flowsTo(getInstanceInput(_, this.getConstructor().getDeclaringType()))
+    )
+  }
+
+  /** Gets a store, for example `outputStream.writeObject(instance)`. */
+  override Expr getAStore() {
+    exists(ClassStoreFlowConfig conf, DataFlow::Node n |
+      serializableStore(n, result) and
+      conf.hasFlow(DataFlow::exprNode(this), n)
+    )
+  }
+}
+
+/** The instantiation of a marshallable class, which can be stored to disk as XML. */
+private class Marshallable extends ClassStore {
+  Marshallable() { this.getConstructor().getDeclaringType() instanceof JAXBElement }
+
+  /** Gets a store, for example `marshaller.marshal(instance)`. */
+  override Expr getAStore() {
+    exists(ClassStoreFlowConfig conf, DataFlow::Node n |
+      marshallableStore(n, result) and
+      conf.hasFlow(DataFlow::exprNode(this), n)
+    )
+  }
+}
+
+/** Gets an input, for example `input` in `instance.password = input`. */
+private Expr getInstanceInput(DataFlow::Node instance, RefType t) {
+  exists(AssignExpr a, FieldAccess fa |
+    instance = DataFlow::getFieldQualifier(fa) and
+    a.getDest() = fa and
+    a.getSource() = result and
+    fa.getField().getDeclaringType() = t
+  |
+    t.getASourceSupertype*() instanceof TypeSerializable or
+    t instanceof JAXBElement
+  )
+}
+
+private class ClassStoreFlowConfig extends DataFlow2::Configuration {
+  ClassStoreFlowConfig() { this = "ClassStoreFlowConfig" }
+
+  override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ClassStore }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(getInstanceInput(sink, _)) or
+    serializableStore(sink, _) or
+    marshallableStore(sink, _)
+  }
+
+  override int fieldFlowBranchLimit() { result = 1 }
+}
+
+private predicate serializableStore(DataFlow::Node instance, Expr store) {
+  exists(MethodAccess m |
+    store = m and
+    m.getMethod() instanceof WriteObjectMethod and
+    instance.asExpr() = m.getArgument(0)
+  )
+}
+
+private predicate marshallableStore(DataFlow::Node instance, Expr store) {
+  exists(MethodAccess m |
+    store = m and
+    m.getMethod() instanceof JAXBMarshalMethod and
+    instance.asExpr() = m.getArgument(0)
+  )
+}

java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll

@@ -0,0 +1,48 @@
+/** Provides classes and predicates to reason about cleartext storage in cookies. */
+
+import java
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.dataflow.DataFlow3
+import semmle.code.java.security.CleartextStorageQuery
+
+private class CookieCleartextStorageSink extends CleartextStorageSink {
+  CookieCleartextStorageSink() { this.asExpr() = cookieInput(_) }
+}
+
+/** The instantiation of a cookie, which can act as storage. */
+class Cookie extends Storable, ClassInstanceExpr {
+  Cookie() {
+    this.getConstructor().getDeclaringType().hasQualifiedName("javax.servlet.http", "Cookie")
+  }
+
+  /** Gets an input, for example `input` in `new Cookie("...", input);`. */
+  override Expr getAnInput() { result = cookieInput(this) }
+
+  /** Gets a store, for example `response.addCookie(cookie);`. */
+  override Expr getAStore() {
+    exists(CookieToStoreFlowConfig conf, DataFlow::Node n |
+      cookieStore(n, result) and
+      conf.hasFlow(DataFlow::exprNode(this), n)
+    )
+  }
+}
+
+private predicate cookieStore(DataFlow::Node cookie, Expr store) {
+  exists(MethodAccess m, Method def |
+    m.getMethod() = def and
+    def.getName() = "addCookie" and
+    def.getDeclaringType().hasQualifiedName("javax.servlet.http", "HttpServletResponse") and
+    store = m and
+    cookie.asExpr() = m.getAnArgument()
+  )
+}
+
+private class CookieToStoreFlowConfig extends DataFlow3::Configuration {
+  CookieToStoreFlowConfig() { this = "CookieToStoreFlowConfig" }
+
+  override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof Cookie }
+
+  override predicate isSink(DataFlow::Node sink) { cookieStore(sink, _) }
+}
+
+private Expr cookieInput(Cookie c) { result = c.getArgument(1) }

java/ql/lib/semmle/code/java/security/CleartextStoragePropertiesQuery.qll

@@ -0,0 +1,62 @@
+/** Provides classes and predicates to reason about cleartext storage in Properties files. */
+
+import java
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.frameworks.Properties
+import semmle.code.java.security.CleartextStorageQuery
+
+private class PropertiesCleartextStorageSink extends CleartextStorageSink {
+  PropertiesCleartextStorageSink() {
+    exists(MethodAccess m |
+      m.getMethod() instanceof PropertiesSetPropertyMethod and this.asExpr() = m.getArgument(1)
+    )
+  }
+}
+
+/** The instantiation of a `Properties` object, which can be stored to disk. */
+class Properties extends Storable, ClassInstanceExpr {
+  Properties() { this.getConstructor().getDeclaringType() instanceof TypeProperty }
+
+  /** Gets an input, for example `input` in `props.setProperty("password", input);`. */
+  override Expr getAnInput() {
+    exists(PropertiesFlowConfig conf, DataFlow::Node n |
+      propertiesInput(n, result) and
+      conf.hasFlow(DataFlow::exprNode(this), n)
+    )
+  }
+
+  /** Gets a store, for example `props.store(outputStream, "...")`. */
+  override Expr getAStore() {
+    exists(PropertiesFlowConfig conf, DataFlow::Node n |
+      propertiesStore(n, result) and
+      conf.hasFlow(DataFlow::exprNode(this), n)
+    )
+  }
+}
+
+private predicate propertiesInput(DataFlow::Node prop, Expr input) {
+  exists(MethodAccess m |
+    m.getMethod() instanceof PropertiesSetPropertyMethod and
+    input = m.getArgument(1) and
+    prop.asExpr() = m.getQualifier()
+  )
+}
+
+private predicate propertiesStore(DataFlow::Node prop, Expr store) {
+  exists(MethodAccess m |
+    m.getMethod() instanceof PropertiesStoreMethod and
+    store = m and
+    prop.asExpr() = m.getQualifier()
+  )
+}
+
+private class PropertiesFlowConfig extends DataFlow::Configuration {
+  PropertiesFlowConfig() { this = "PropertiesFlowConfig" }
+
+  override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof Properties }
+
+  override predicate isSink(DataFlow::Node sink) {
+    propertiesInput(sink, _) or
+    propertiesStore(sink, _)
+  }
+}

java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll

@@ -0,0 +1,101 @@
+/** Provides classes and predicates to reason about cleartext storage vulnerabilities. */
+
+import java
+private import semmle.code.java.dataflow.DataFlow4
+private import semmle.code.java.dataflow.TaintTracking
+private import semmle.code.java.security.SensitiveActions
+
+/** A sink representing persistent storage that saves data in clear text. */
+abstract class CleartextStorageSink extends DataFlow::Node { }
+
+/** A sanitizer for flows tracking sensitive data being stored in persistent storage. */
+abstract class CleartextStorageSanitizer extends DataFlow::Node { }
+
+/** An additional taint step for sensitive data flowing into cleartext storage. */
+class CleartextStorageAdditionalTaintStep extends Unit {
+  abstract predicate step(DataFlow::Node n1, DataFlow::Node n2);
+}
+
+/** Class for expressions that may represent 'sensitive' information */
+class SensitiveSource extends Expr instanceof SensitiveExpr {
+  /** Holds if this source flows to the `sink`. */
+  predicate flowsTo(Expr sink) {
+    exists(SensitiveSourceFlowConfig conf |
+      conf.hasFlow(DataFlow::exprNode(this), DataFlow::exprNode(sink))
+    )
+  }
+}
+
+/**
+ *  Class representing entities that may be stored/written, with methods
+ *  for finding values that are stored within them, and cases
+ *  of the entity being stored.
+ */
+abstract class Storable extends Call {
+  /** Gets an "input" that is stored in an instance of this class. */
+  abstract Expr getAnInput();
+
+  /** Gets an expression where an instance of this class is stored (e.g. to disk). */
+  abstract Expr getAStore();
+}
+
+private class SensitiveSourceFlowConfig extends TaintTracking::Configuration {
+  SensitiveSourceFlowConfig() { this = "SensitiveSourceFlowConfig" }
+
+  override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SensitiveExpr }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof CleartextStorageSink }
+
+  override predicate isSanitizer(DataFlow::Node sanitizer) {
+    sanitizer instanceof CleartextStorageSanitizer
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
+    any(CleartextStorageAdditionalTaintStep c).step(n1, n2)
+  }
+}
+
+private class DefaultCleartextStorageSanitizer extends CleartextStorageSanitizer {
+  DefaultCleartextStorageSanitizer() {
+    this.getType() instanceof NumericType or
+    this.getType() instanceof BooleanType or
+    exists(EncryptedValueFlowConfig conf | conf.hasFlow(_, this))
+  }
+}
+
+/**
+ * Method call for encrypting sensitive information. As there are various implementations of
+ * encryption (reversible and non-reversible) from both JDK and third parties, this class simply
+ * checks method name to take a best guess to reduce false positives.
+ */
+private class EncryptedSensitiveMethodAccess extends MethodAccess {
+  EncryptedSensitiveMethodAccess() {
+    this.getMethod().getName().toLowerCase().matches(["%encrypt%", "%hash%", "%digest%"])
+  }
+}
+
+/** Flow configuration for encryption methods flowing to inputs of persistent storage. */
+private class EncryptedValueFlowConfig extends DataFlow4::Configuration {
+  EncryptedValueFlowConfig() { this = "EncryptedValueFlowConfig" }
+
+  override predicate isSource(DataFlow::Node src) {
+    src.asExpr() instanceof EncryptedSensitiveMethodAccess
+  }
+
+  override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof SensitiveExpr }
+}
+
+/** A taint step for `EditText.toString` in Android. */
+private class AndroidEditTextCleartextStorageStep extends CleartextStorageAdditionalTaintStep {
+  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
+    // EditText.getText() return type is parsed as `Object`, so we need to
+    // add a taint step for `Object.toString` to model `editText.getText().toString()`
+    exists(MethodAccess ma, Method m |
+      ma.getMethod() = m and
+      m.getDeclaringType() instanceof TypeObject and
+      m.hasName("toString")
+    |
+      n1.asExpr() = ma.getQualifier() and n2.asExpr() = ma
+    )
+  }
+}

java/ql/src/Security/CWE/CWE-312/CleartextStorageClass.ql

@@ -12,15 +12,12 @@
  */
 
 import java
-import SensitiveStorage
+import semmle.code.java.security.CleartextStorageClassQuery
 
 from SensitiveSource data, ClassStore s, Expr input, Expr store
 where
   input = s.getAnInput() and
   store = s.getAStore() and
-  data.flowsToCached(input) and
-  // Exclude results in test code.
-  not testMethod(store.getEnclosingCallable()) and
-  not testMethod(data.getEnclosingCallable())
+  data.flowsTo(input)
 select store, "Storable class $@ containing $@ is stored here. Data was added $@.", s, s.toString(),
   data, "sensitive data", input, "here"

java/ql/src/Security/CWE/CWE-312/CleartextStorageCookie.ql

@@ -11,15 +11,12 @@
  */
 
 import java
-import SensitiveStorage
+import semmle.code.java.security.CleartextStorageCookieQuery
 
 from SensitiveSource data, Cookie s, Expr input, Expr store
 where
   input = s.getAnInput() and
   store = s.getAStore() and
-  data.flowsToCached(input) and
-  // Exclude results in test code.
-  not testMethod(store.getEnclosingCallable()) and
-  not testMethod(data.getEnclosingCallable())
+  data.flowsTo(input)
 select store, "Cookie $@ containing $@ is stored here. Data was added $@.", s, s.toString(), data,
   "sensitive data", input, "here"

java/ql/src/Security/CWE/CWE-312/CleartextStorageProperties.ql

@@ -11,15 +11,12 @@
  */
 
 import java
-import SensitiveStorage
+import semmle.code.java.security.CleartextStoragePropertiesQuery
 
 from SensitiveSource data, Properties s, Expr input, Expr store
 where
   input = s.getAnInput() and
   store = s.getAStore() and
-  data.flowsToCached(input) and
-  // Exclude results in test code.
-  not testMethod(store.getEnclosingCallable()) and
-  not testMethod(data.getEnclosingCallable())
+  data.flowsTo(input)
 select store, "'Properties' class $@ containing $@ is stored here. Data was added $@.", s,
   s.toString(), data, "sensitive data", input, "here"