Skip to content

Java: Refactor Cleartext Storage queries #6493

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 105 additions & 0 deletions java/ql/lib/semmle/code/java/security/CleartextStorageClassQuery.qll
Original file line number Diff line number Diff line change
@@ -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)
)
}
Original file line number Diff line number Diff line change
@@ -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) }
Original file line number Diff line number Diff line change
@@ -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, _)
}
}
101 changes: 101 additions & 0 deletions java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll
Original file line number Diff line number Diff line change
@@ -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
)
}
}
7 changes: 2 additions & 5 deletions java/ql/src/Security/CWE/CWE-312/CleartextStorageClass.ql
Original file line number Diff line number Diff line change
Expand Up @@ -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"
7 changes: 2 additions & 5 deletions java/ql/src/Security/CWE/CWE-312/CleartextStorageCookie.ql
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Loading