Skip to content
Draft
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `py/clear-text-logging-sensitive-data`, `py/clear-text-storage-sensitive-data`, and `py/weak-sensitive-data-hashing` queries no longer propagate sensitive data through resolved configuration lookups whose only value selectors are concrete, non-sensitive `section` and `key` arguments. Calls with sensitive or dynamic names, or with additional value arguments, remain unchanged.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
private import python
private import semmle.python.controlflow.internal.Cfg as Cfg
private import semmle.python.dataflow.new.DataFlow
private import semmle.python.dataflow.new.internal.DataFlowDispatch as DataFlowDispatch
// Need to import `semmle.python.Frameworks` since frameworks can extend `SensitiveDataSource::Range`
private import semmle.python.Frameworks
private import codeql.concepts.internal.SensitiveDataHeuristics as SensitiveDataHeuristics
Expand Down Expand Up @@ -84,6 +85,87 @@ private module SensitiveDataModeling {
sensitiveFunction(DataFlow::TypeTracker::end(), classification).flowsTo(result)
}

/** Gets the unique string literal passed to `parameterName`, if all local sources agree. */
private string constantArgument(DataFlowDispatch::NormalCall call, string parameterName) {
exists(
DataFlowDispatch::ArgumentPosition argumentPosition,
DataFlowDispatch::ParameterPosition parameterPosition, DataFlow::ArgumentNode argument,
DataFlow::LocalSourceNode representative
|
DataFlowDispatch::parameterMatch(parameterPosition, argumentPosition) and
call.getCallable().getParameter(parameterPosition).getParameter().getName() = parameterName and
argument = call.getArgument(argumentPosition) and
representative = argument.getALocalSource() and
result = representative.asExpr().(StringLiteral).getText() and
forall(DataFlow::LocalSourceNode source | source = argument.getALocalSource() |
source.asExpr().(StringLiteral).getText() = result
)
)
}

private predicate resolvedConfigurationCall(
DataFlowDispatch::NormalCall call, string section, string key
) {
section = constantArgument(call, "section") and
key = constantArgument(call, "key") and
not callNameIndicatesSensitiveData(call) and
onlyConfigurationSelectorArguments(call)
}

private predicate onlyConfigurationSelectorArguments(DataFlowDispatch::NormalCall call) {
forall(DataFlowDispatch::ArgumentPosition argumentPosition |
exists(call.getArgument(argumentPosition))
|
exists(DataFlowDispatch::ParameterPosition parameterPosition |
DataFlowDispatch::parameterMatch(parameterPosition, argumentPosition) and
(
parameterPosition.isSelf()
or
call.getCallable().getParameter(parameterPosition).getParameter().getName() in [
"section", "key"
]
)
)
)
}

private predicate callNameIndicatesSensitiveData(DataFlowDispatch::NormalCall call) {
nameIndicatesSensitiveData(call.getCallable().getScope().(Function).getName())
or
nameIndicatesSensitiveData(call.getNode().(Cfg::CallNode).getFunction().(Cfg::NameNode).getId())
or
nameIndicatesSensitiveData(call.getNode()
.(Cfg::CallNode)
.getFunction()
.(Cfg::AttrNode)
.getName())
}

bindingset[name]
private predicate configurationSelectorIndicatesSensitiveData(string name) {
// File, path, and URL suffixes do not make configuration selectors safe: they
// commonly identify an indirect secret or a value that embeds credentials.
name.regexpMatch(maybeSensitiveRegexp(_))
or
name.regexpMatch("(?is).*(^|[_-])(secret|auth|salt|bearer|key(tab)?|token|cred(ential)?|conn(ect(ion)?)?|"
+ "backend|dsn|url|uri|args|kwargs)([_-]|$).*")
}

/**
* Holds if every resolved target for `node` has concrete `section` and `key`
* arguments whose names do not indicate sensitive data.
*/
predicate knownNonSensitiveConfigurationValue(DataFlow::Node node) {
exists(DataFlowDispatch::NormalCall call | call.getNode() = node.asCfgNode()) and
forall(DataFlowDispatch::NormalCall call | call.getNode() = node.asCfgNode() |
exists(string section, string key |
resolvedConfigurationCall(call, section, key) and
not configurationSelectorIndicatesSensitiveData(section) and
not configurationSelectorIndicatesSensitiveData(key)
)
)
}

/**
* Gets a reference (in local scope) to a string constant that, if used as the key in
* a lookup, indicates the presence of sensitive data with `classification`.
Expand Down Expand Up @@ -336,4 +418,16 @@ private module SensitiveDataModeling {

predicate sensitiveDataExtraStepForCalls = SensitiveDataModeling::extraStepForCalls/2;

/**
* Holds if `node` is a resolved configuration lookup whose only inputs are its
* receiver and concrete, non-sensitive section and key names.
*
* This predicate is intended for use as a sensitive-data barrier. It blocks all
* flow through the lookup result, so calls with additional value arguments are
* deliberately excluded.
*/
predicate isKnownNonSensitiveConfigurationLookup(DataFlow::Node node) {
SensitiveDataModeling::knownNonSensitiveConfigurationValue(node)
}

predicate sensitiveLookupStringConst = SensitiveDataModeling::sensitiveLookupStringConst/1;
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ module CleartextLogging {
*/
abstract class Sanitizer extends DataFlow::Node { }

private class KnownNonSensitiveConfigurationLookupSanitizer extends Sanitizer {
KnownNonSensitiveConfigurationLookupSanitizer() { isKnownNonSensitiveConfigurationLookup(this) }
}

/**
* A source of sensitive data, considered as a flow source.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ module CleartextStorage {
*/
abstract class Sanitizer extends DataFlow::Node { }

private class KnownNonSensitiveConfigurationLookupSanitizer extends Sanitizer {
KnownNonSensitiveConfigurationLookupSanitizer() { isKnownNonSensitiveConfigurationLookup(this) }
}

/**
* A source of sensitive data, considered as a flow source.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ module NormalHashFunction {
*/
abstract class Sanitizer extends DataFlow::Node { }

private class KnownNonSensitiveConfigurationLookupSanitizer extends Sanitizer {
KnownNonSensitiveConfigurationLookupSanitizer() { isKnownNonSensitiveConfigurationLookup(this) }
}

/**
* A source of sensitive data, considered as a flow source.
*/
Expand Down Expand Up @@ -117,6 +121,10 @@ module ComputationallyExpensiveHashFunction {
*/
abstract class Sanitizer extends DataFlow::Node { }

private class KnownNonSensitiveConfigurationLookupSanitizer extends Sanitizer {
KnownNonSensitiveConfigurationLookupSanitizer() { isKnownNonSensitiveConfigurationLookup(this) }
}

/**
* A source of passwords, considered as a flow source.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ module SensitiveUseConfig implements DataFlow::ConfigSig {
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
sensitiveDataExtraStepForCalls(node1, node2)
}

predicate isBarrier(DataFlow::Node node) { isKnownNonSensitiveConfigurationLookup(node) }
}

module SensitiveUseFlow = TaintTracking::Global<SensitiveUseConfig>;
Expand Down
63 changes: 63 additions & 0 deletions python/ql/test/library-tests/dataflow/sensitive-data/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,66 @@ def call_wrapper(func):

# since we have precise dictionary content, other items of the config are not tainted
print(_config["sleep_timer"])


# ------------------------------------------------------------------------------
# cross-talk through configuration getters.
# ------------------------------------------------------------------------------

class Configuration:
def _get_secret_option(self, section, key):
if self.is_sensitive(section, key):
return load_secret_value() # $ SensitiveDataSource=secret
return None

def get(self, section, key, fallback="default"):
value = self._get_secret_option(section, key) # $ SensitiveDataSource=secret
if value is not None:
return value
return fallback

def getlist(self, section, key):
return self.get(section, key).split(",")

def get_mandatory_list_value(self, section, key):
return self.getlist(section, key)


configuration = Configuration()

executor_name = configuration.get_mandatory_list_value("core", "EXECUTOR")[0]
print(executor_name)

# A second Airflow-shaped path carries a non-sensitive directory through a helper.
dags_folder = configuration.get_mandatory_list_value("core", "DAGS_FOLDER")[0]


def find_path_from_directory(base_dir_path):
print(base_dir_path)


find_path_from_directory(dags_folder)

# Concrete sensitive keys remain conservative.
value2 = configuration.get_mandatory_list_value("database", "PASSWORD")[0]
print(value2) # $ SensitiveUse=secret

# Configuration key names that can hold or locate secrets remain conservative.
value_with_key_suffix = configuration.get_mandatory_list_value("crypto", "FERNET_KEY")[0]
print(value_with_key_suffix) # $ SensitiveUse=secret

value_with_path_suffix = configuration.get_mandatory_list_value("smtp", "PASSWORD_FILE")[0]
print(value_with_path_suffix) # $ SensitiveUse=secret

# Dynamic keys remain conservative.
dynamic_key = get_key()
dynamic_value = configuration.get_mandatory_list_value("core", dynamic_key)[0]
print(dynamic_value) # $ SensitiveUse=secret

# A sensitive callee name remains a source even with non-sensitive selector names.
value3 = configuration._get_secret_option("core", "EXECUTOR") # $ SensitiveDataSource=secret
print(value3) # $ SensitiveUse=secret

# Additional value arguments prevent the call result from becoming a barrier.
value4 = configuration.get("core", "EXECUTOR", get_password()) # $ SensitiveDataSource=password
print(value4) # $ SensitiveUse=password SensitiveUse=secret
Loading