Skip to content

Swift: Replace two additional taint steps with implicit reads #14357

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.

Sign up for GitHub

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 3 commits into from
Oct 3, 2023
Merged

Swift: Replace two additional taint steps with implicit reads #14357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
27bdee8
Swift: Replace additional taint step with implict read.
geoffw0 Aug 14, 2023
81b358a
Swift: Replace a similar additional taint step in another query.
geoffw0 Aug 14, 2023
bbd3c66
Swift: Update for CollectionContent.
geoffw0 Oct 2, 2023
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
8 changes: 1 addition & 7 deletions swift/ql/lib/codeql/swift/security/CleartextStorageDatabaseExtensions.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,9 @@ private class CleartextStorageDatabaseDefaultBarrier extends CleartextStorageDat
/**
* An additional taint step for cleartext database storage vulnerabilities.
*/
private class CleartextStorageDatabaseArrayAdditionalFlowStep extends CleartextStorageDatabaseAdditionalFlowStep
private class CleartextStorageDatabaseFieldsAdditionalFlowStep extends CleartextStorageDatabaseAdditionalFlowStep
{
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// needed until we have proper content flow through arrays.
exists(ArrayExpr arr |
nodeFrom.asExpr() = arr.getAnElement() and
nodeTo.asExpr() = arr
)
or
// if an object is sensitive, its fields are always sensitive
// (this is needed because the sensitive data sources are in a sense
// approximate; for example we might identify `passwordBox` as a source,
Expand Down
5 changes: 5 additions & 0 deletions swift/ql/lib/codeql/swift/security/CleartextStorageDatabaseQuery.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ module CleartextStorageDatabaseConfig implements DataFlow::ConfigSig {
cx.asNominalTypeDecl() = d and
c.getAReadContent().(DataFlow::Content::FieldContent).getField() = cx.getAMember()
)
or
// flow out from array elements of at the sink,
// for example in `database.allStatements(sql: "", arguments: [sensitive])`.
isSink(node) and
c.getAReadContent() instanceof DataFlow::Content::CollectionContent
}
}

Expand Down
13 changes: 0 additions & 13 deletions swift/ql/lib/codeql/swift/security/CommandInjectionExtensions.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,6 @@ class CommandInjectionAdditionalFlowStep extends Unit {
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
}

/**
* An additional taint step for command injection vulnerabilities.
*/
private class CommandInjectionArrayAdditionalFlowStep extends CommandInjectionAdditionalFlowStep {
override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// needed until we have proper content flow through arrays.
exists(ArrayExpr arr |
nodeFrom.asExpr() = arr.getAnElement() and
nodeTo.asExpr() = arr
)
}
}

/**
* A sink defined in a CSV model.
*/
Expand Down
6 changes: 6 additions & 0 deletions swift/ql/lib/codeql/swift/security/CommandInjectionQuery.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ module CommandInjectionConfig implements DataFlow::ConfigSig {
predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
any(CommandInjectionAdditionalFlowStep s).step(nodeFrom, nodeTo)
}

predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
// flow out from array elements of at the sink, for example in `task.arguments = [tainted]`.
isSink(node) and
c.getAReadContent() instanceof DataFlow::Content::CollectionContent
}
}

/**
Expand Down
Loading