Skip to content

Swift: Queries for CWE-311 (originally CWE-200) #10061

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 23 commits into from
Sep 1, 2022
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
c1be060
Swift: Create query + test stubs.
geoffw0 Aug 11, 2022
dd51b7f
Swift: Add many tests.
geoffw0 Aug 11, 2022
1c52836
Swift: Additional test cases.
geoffw0 Aug 16, 2022
2aa6dd2
Swift: Make tests more accurate and don't use 'pwd' as a variable nam…
geoffw0 Aug 23, 2022
dacb7f5
Swift: Add a SensitiveExprs lib (and test it).
geoffw0 Aug 23, 2022
0cd2efc
Swift: CleartextTransmission query.
geoffw0 Aug 23, 2022
456ab98
Swift: Fix duplicate results.
geoffw0 Aug 23, 2022
3126fb9
Swift: Core Data support.
geoffw0 Aug 24, 2022
698a9e2
Swift: Realm database support.
geoffw0 Aug 24, 2022
2690732
Swift: Special cases to get taint flow working.
geoffw0 Aug 24, 2022
9a53a40
Swift: Qhelp and examples for both queries.
geoffw0 Aug 25, 2022
926da4b
Swift: Query descriptions and metadata.
geoffw0 Aug 25, 2022
7fd64f1
Swift: Make QL-for-QL happy.
geoffw0 Aug 25, 2022
2d57786
Merge branch 'main' into cleartext
geoffw0 Aug 25, 2022
645364e
Update swift/ql/src/queries/Security/CWE-311/CleartextStorageDatabase…
geoffw0 Aug 30, 2022
4984d8f
Apply suggestions from code review
geoffw0 Aug 30, 2022
430a8e1
Swift: Fix issues.
geoffw0 Aug 30, 2022
3e4a6be
Swift: Add missing test annotations.
geoffw0 Aug 30, 2022
60fad4d
Merge remote-tracking branch 'upstream/main' into swiftcleanup
geoffw0 Aug 31, 2022
c0bc0d7
Swift: Accept test changes after merging main (again).
geoffw0 Aug 31, 2022
129ed42
Swift: Use allowImplicitRead as a better solution replacing one of th…
geoffw0 Aug 31, 2022
92a927e
Swift: Remove special case (no longer required).
geoffw0 Sep 1, 2022
d3250a7
Swift: Fix finding base classes.
geoffw0 Sep 1, 2022
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
156 changes: 156 additions & 0 deletions swift/ql/lib/codeql/swift/security/SensitiveExprs.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
/**
* Provides classes for heuristically identifying expressions that contain
* 'sensitive' data, meaning that they contain or return a password or other
* credential, or sensitive private information.
*/

import swift

private newtype TSensitiveDataType =
TCredential() or
TPrivateInfo()

/**
* A type of sensitive expression.
*/
abstract class SensitiveDataType extends TSensitiveDataType {
abstract string toString();

/**
* Gets a regexp for identifying expressions of this type.
*/
abstract string getRegexp();
}

/**
* The type of sensitive expression for passwords and other credentials.
*/
class SensitiveCredential extends SensitiveDataType, TCredential {
override string toString() { result = "credential" }

override string getRegexp() {
result = ".*(password|passwd|accountid|account.?key|accnt.?key|license.?key|trusted).*"
}
}

/**
* The type of sensitive expression for private information.
*/
class SensitivePrivateInfo extends SensitiveDataType, TPrivateInfo {
override string toString() { result = "private information" }

override string getRegexp() {
result =
".*(" +
// Inspired by the list on https://cwe.mitre.org/data/definitions/359.html
// Government identifiers, such as Social Security Numbers
"social.?security|national.?insurance|" +
// Contact information, such as home addresses
"post.?code|zip.?code|home.?address|" +
// and telephone numbers
"telephone|home.?phone|mobile|fax.?no|fax.?number|" +
// Geographic location - where the user is (or was)
"latitude|longitude|" +
// Financial data - such as credit card numbers, salary, bank accounts, and debts
"credit.?card|debit.?card|salary|bank.?account|" +
// Communications - e-mail addresses, private e-mail messages, SMS text messages, chat logs, etc.
"email|" +
// Health - medical conditions, insurance status, prescription records
"birthday|birth.?date|date.?of.?birth|medical|" +
// Relationships - work and family
"employer|spouse" +
// ---
").*"
}
}

/**
* A regexp string to identify variables etc that might be safe because they
* contain hashed or encrypted data, or are only a reference to data that is
* actually stored elsewhere.
*/
private string regexpProbablySafe() { result = ".*(hash|crypt|file|path|invalid).*" }

/**
* A `VarDecl` that might be used to contain sensitive data.
*/
private class SensitiveVarDecl extends VarDecl {
SensitiveDataType sensitiveType;

SensitiveVarDecl() { this.getName().toLowerCase().regexpMatch(sensitiveType.getRegexp()) }

predicate hasInfo(string label, SensitiveDataType type) {
label = this.getName() and
sensitiveType = type
}
}

/**
* An `AbstractFunctionDecl` that might be used to contain sensitive data.
*/
private class SensitiveFunctionDecl extends AbstractFunctionDecl {
SensitiveDataType sensitiveType;

SensitiveFunctionDecl() { this.getName().toLowerCase().regexpMatch(sensitiveType.getRegexp()) }

predicate hasInfo(string label, SensitiveDataType type) {
label = this.getName() and
sensitiveType = type
}
}

/**
* An `Argument` that might be used to contain sensitive data.
*/
private class SensitiveArgument extends Argument {
SensitiveDataType sensitiveType;

SensitiveArgument() { this.getLabel().toLowerCase().regexpMatch(sensitiveType.getRegexp()) }

predicate hasInfo(string label, SensitiveDataType type) {
label = this.getLabel() and
sensitiveType = type
}
}

/**
* An expression whose value might be sensitive data.
*/
class SensitiveExpr extends Expr {
string label;
SensitiveDataType sensitiveType;

SensitiveExpr() {
// variable reference
this.(DeclRefExpr).getDecl().(SensitiveVarDecl).hasInfo(label, sensitiveType)
or
// member variable reference
this.(MemberRefExpr).getMember().(SensitiveVarDecl).hasInfo(label, sensitiveType)
or
// function call
this.(ApplyExpr).getStaticTarget().(SensitiveFunctionDecl).hasInfo(label, sensitiveType)
or
// sensitive argument
exists(SensitiveArgument a |
a.hasInfo(label, sensitiveType) and
a.getExpr() = this
)
}

/**
* Gets the label associated with this expression.
*/
string getLabel() { result = label }

/**
* Gets the type of sensitive expression this is.
*/
SensitiveDataType getSensitiveType() { result = sensitiveType }

/**
* Holds if this sensitive expression might be safe because it contains
* hashed or encrypted data, or is only a reference to data that is stored
* elsewhere.
*/
predicate isProbablySafe() { label.toLowerCase().regexpMatch(regexpProbablySafe()) }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>

<p>Sensitive information that is stored unencrypted in a database is accessible to an attacker who gains access to that database. For example the information could be accessed by any process or user in a rooted device, or exposed through another vulnerability.</p>

</overview>
<recommendation>

<p>Either encrypt the entire database, or ensure that each piece of sensitive information is encrypted before being stored. In general, decrypt sensitive information only at the point where it is necessary for it to be used in cleartext. Avoid storing sensitive information at all if you do not need to keep it.</p>

</recommendation>
<example>

<p>The following example shows three cases of storing information using the Core Data library. In the 'BAD' case, the data that is stored is sensitive (a credit card number) and is not encrypted. In the 'GOOD' cases, the data is either not sensitive or is protected with encryption.</p>

<sample src="CleartextStorageDatabase.swift" />

</example>
<references>

<li>
OWASP Top 10:2021:
<a href="https://owasp.org/Top10/A02_2021-Cryptographic_Failures/">A02:2021 � Cryptographic Failures</a>
</li>

</references>
</qhelp>
109 changes: 109 additions & 0 deletions swift/ql/src/queries/Security/CWE-311/CleartextStorageDatabase.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/**
* @name Cleartext storage of sensitive information in a local database
* @description Storing sensitive information in a non-encrypted
* database can expose it to an attacker.
* @kind path-problem
* @problem.severity warning
* @security-severity 7.5
* @precision medium
* @id swift/cleartext-storage-database
* @tags security
* external/cwe/cwe-312
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be CWE 311?

Suggested change
* external/cwe/cwe-312
* external/cwe/cwe-311

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, CWE-312 is a child of CWE-311. The relevant part of the hierarchy looks something like this:

CWE-693: Protection Mechanism Failure
 |-- ...
 \-- CWE-311: Missing Encryption of Sensitive Data
      |-- CWE-312: Cleartext Storage of Sensitive Information
      \-- CWE-319: Cleartext Transmission of Sensitive Information

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying 🙇🏻‍♀️

*/

import swift
import codeql.swift.security.SensitiveExprs
import codeql.swift.dataflow.DataFlow
import codeql.swift.dataflow.TaintTracking
import DataFlow::PathGraph

/**
* An `Expr` that is stored in a local database.
*/
abstract class Stored extends Expr { }

/**
* An `Expr` that is stored with the Core Data library.
*/
class CoreDataStore extends Stored {
CoreDataStore() {
// `content` arg to `NWConnection.send` is a sink
exists(ClassDecl c, AbstractFunctionDecl f, CallExpr call |
c.getName() = "NSManagedObject" and
c.getAMember() = f and
f.getName() = ["setValue(_:forKey:)", "setPrimitiveValue(_:forKey:)"] and
call.getFunction().(ApplyExpr).getStaticTarget() = f and
call.getArgument(0).getExpr() = this
)
}
}

/**
* An `Expr` that is stored with the Realm database library.
*/
class RealmStore extends Stored {
RealmStore() {
// `object` arg to `Realm.add` is a sink
exists(ClassDecl c, AbstractFunctionDecl f, CallExpr call |
c.getName() = "Realm" and
c.getAMember() = f and
f.getName() = ["add(_:update:)"] and
call.getFunction().(ApplyExpr).getStaticTarget() = f and
call.getArgument(0).getExpr() = this
)
or
// `value` arg to `Realm.create` is a sink
exists(ClassDecl c, AbstractFunctionDecl f, CallExpr call |
c.getName() = "Realm" and
c.getAMember() = f and
f.getName() = ["create(_:value:update:)"] and
call.getFunction().(ApplyExpr).getStaticTarget() = f and
call.getArgument(1).getExpr() = this
)
}
}

/**
* A taint configuration from sensitive information to expressions that are
* transmitted over a network.
*/
class CleartextStorageConfig extends TaintTracking::Configuration {
CleartextStorageConfig() { this = "CleartextStorageConfig" }

override predicate isSource(DataFlow::Node node) {
exists(SensitiveExpr e |
node.asExpr() = e and
not e.isProbablySafe()
)
}

override predicate isSink(DataFlow::Node node) { node.asExpr() instanceof Stored }

override predicate isSanitizerIn(DataFlow::Node node) {
// make sources barriers so that we only report the closest instance
isSource(node)
}

override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
// TODO: the following special case flows are required to catch any of the Realm test
// cases, I hope we'll be able to remove them once we have field flow???
// flow out from field accesses, i.e. `a.b` -> `a`
exists(MemberRefExpr m |
node1.asExpr() = m and // `a.b`
node2.asExpr() = m.getBaseExpr() // `a`
)
or
// flow through assignment (!)
exists(AssignExpr ae |
node1.asExpr() = ae.getSource() and
node2.asExpr() = ae.getDest()
)
}
}

from CleartextStorageConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode
where config.hasFlowPath(sourceNode, sinkNode)
select sinkNode.getNode(), sourceNode, sinkNode,
"This operation stores '" + sinkNode.getNode().toString() +
"' in a database. It may contain unencrypted sensitive data from $@", sourceNode,
sourceNode.getNode().toString()
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@

func storeMyData(databaseObject : NSManagedObject, faveSong : String, creditCardNo : String) {
// ...

// GOOD: not sensitive information
databaseObject.setValue(faveSong, forKey: "myFaveSong")

// BAD: sensitive information saved in cleartext
databaseObject.setValue(creditCardNo, forKey: "myCreditCardNo")

// GOOD: encrypted sensitive information saved
databaseObject.setValue(encrypt(creditCardNo), forKey: "myCreditCardNo")

// ...
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>

<p>Sensitive information that is transmitted without encryption may be accessible to an attacker.</p>

</overview>
<recommendation>

<p>Ensure that sensitive information is always encrypted before being transmitted over the network. In general, decrypt sensitive information only at the point where it is necessary for it to be used in cleartext. Avoid transmitting sensitive information when it is not necessary to.</p>

</recommendation>
<example>

<p>The following example shows three cases of transmitting information. In the 'BAD' case, the data transmitted is sensitive (a credit card number) and is not encrypted. In the 'GOOD' cases, the data is either not sensitive or is protected with encryption.</p>

<sample src="CleartextTransmission.swift" />

</example>
<references>

<li>
OWASP Top 10:2021:
<a href="https://owasp.org/Top10/A02_2021-Cryptographic_Failures/">A02:2021 � Cryptographic Failures</a>
</li>

</references>
</qhelp>
Loading