Files
codeql/python/ql/src/Security/CWE-798/HardcodedCredentials.ql
yoff f9398546a4 Python: remove getAFlowNode() — bridge AST→CFG only via CFG-side getNode()
Option 2: eliminates the AST→CFG bridge from the AST layer. Previously
'AstNode.getAFlowNode()' returned a 'ControlFlowNode' from the legacy
'Flow.qll' CFG via 'py_flow_bb_node' — this hardcoded the AST to know
about the legacy CFG, preventing files from cleanly switching to the
new shared CFG.

Removes:
  * 'AstNode.getAFlowNode()' from 'AstExtended.qll'
  * Type-narrowing overrides on 'Attribute' / 'Subscript' / 'Call' /
    'IfExp' / 'Name' / 'NameConstant' / 'ImportMember' (in Exprs.qll
    and Import.qll)

Rewrites ~130 call sites across 'python/ql/lib/' and 'python/ql/src/'
to bridge from the CFG side instead:

  Before:  node = expr.getAFlowNode()
  After:   node.getNode() = expr

  Before:  expr.getAFlowNode().(DefinitionNode).getValue()
  After:   exists(DefinitionNode d | d.getNode() = expr | d.getValue())

  Before:  cn.operands(const.getAFlowNode(), op, x)
  After:   exists(ControlFlowNode c | c.getNode() = const | cn.operands(c, op, x))

This is semantically a no-op — both forms are duals of the same predicate.
Verified by passing all library tests:
  * 64 dataflow tests
  * 28 ControlFlow + dataflow-new-ssa tests
  * 1 essa SSA-compute test
  * 93 tests total in the focused suite

Once committed, files that want to switch from the legacy 'Flow' CFG
to the new 'Cfg' facade only need to change their imports — the
bridge sites are CFG-side and respect whichever ControlFlowNode is in
scope.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-05-28 21:09:46 +00:00

147 lines
4.7 KiB
Plaintext

/**
* @name Hard-coded credentials
* @description Credentials are hard coded in the source code of the application.
* @kind path-problem
* @problem.severity error
* @security-severity 9.8
* @precision low
* @id py/hardcoded-credentials
* @tags security
* external/cwe/cwe-259
* external/cwe/cwe-321
* external/cwe/cwe-798
*/
import python
import semmle.python.dataflow.new.DataFlow
import semmle.python.dataflow.new.TaintTracking
import semmle.python.filters.Tests
private import semmle.python.dataflow.new.internal.DataFlowDispatch as DataFlowDispatch
private import semmle.python.dataflow.new.internal.Builtins::Builtins as Builtins
private import semmle.python.frameworks.data.ModelsAsData
bindingset[char, fraction]
predicate fewer_characters_than(StringLiteral str, string char, float fraction) {
exists(string text, int chars |
text = str.getText() and
chars = count(int i | text.charAt(i) = char)
|
/* Allow one character */
chars = 1 or
chars < text.length() * fraction
)
}
predicate possible_reflective_name(string name) {
any(Function f).getName() = name
or
any(Class c).getName() = name
or
any(Module m).getName() = name
or
exists(Builtins::likelyBuiltin(name))
}
int char_count(StringLiteral str) { result = count(string c | c = str.getText().charAt(_)) }
predicate capitalized_word(StringLiteral str) { str.getText().regexpMatch("[A-Z][a-z]+") }
predicate format_string(StringLiteral str) { str.getText().matches("%{%}%") }
predicate maybeCredential(ControlFlowNode f) {
/* A string that is not too short and unlikely to be text or an identifier. */
exists(StringLiteral str | str = f.getNode() |
/* At least 10 characters */
str.getText().length() > 9 and
/* Not too much whitespace */
fewer_characters_than(str, " ", 0.05) and
/* or underscores */
fewer_characters_than(str, "_", 0.2) and
/* Not too repetitive */
exists(int chars | chars = char_count(str) |
chars > 15 or
chars * 3 > str.getText().length() * 2
) and
not possible_reflective_name(str.getText()) and
not capitalized_word(str) and
not format_string(str)
)
or
/* Or, an integer with over 32 bits */
exists(IntegerLiteral lit | f.getNode() = lit |
not exists(lit.getValue()) and
/* Not a set of flags or round number */
not lit.getN().matches("%00%")
)
}
class HardcodedValueSource extends DataFlow::Node {
HardcodedValueSource() { maybeCredential(this.asCfgNode()) }
}
class CredentialSink extends DataFlow::Node {
CredentialSink() {
exists(string s | s.matches("credentials-%") |
// Actual sink-type will be things like `credentials-password` or `credentials-username`
ModelOutput::sinkNode(this, s)
)
or
exists(string name |
name.regexpMatch(getACredentialRegex()) and
not name.matches("%file")
|
exists(DataFlowDispatch::ArgumentPosition pos | pos.isKeyword(name) |
this.(DataFlow::ArgumentNode).argumentOf(_, pos)
)
or
exists(Keyword k | k.getArg() = name and this.getNode() = k.getValue().asCfgNode())
or
exists(CompareNode cmp, NameNode n | n.getId() = name |
cmp.operands(this.asCfgNode(), any(Eq eq), n)
or
cmp.operands(n, any(Eq eq), this.asCfgNode())
)
)
}
}
class CredentialSanitizer extends DataFlow::Node {
CredentialSanitizer() {
exists(string s | s.matches("credentials-%") |
// Whatever the string, this will sanitize flow to all credential sinks.
ModelOutput::barrierNode(this, s)
)
}
}
/**
* Gets a regular expression for matching names of locations (variables, parameters, keys) that
* indicate the value being held is a credential.
*/
private string getACredentialRegex() {
result = "(?i).*pass(wd|word|code|phrase)(?!.*question).*" or
result = "(?i).*(puid|username|userid).*" or
result = "(?i).*(cert)(?!.*(format|name)).*"
}
private module HardcodedCredentialsConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof HardcodedValueSource }
predicate isSink(DataFlow::Node sink) { sink instanceof CredentialSink }
predicate isBarrier(DataFlow::Node node) { node instanceof CredentialSanitizer }
predicate observeDiffInformedIncrementalMode() { any() }
}
module HardcodedCredentialsFlow = TaintTracking::Global<HardcodedCredentialsConfig>;
import HardcodedCredentialsFlow::PathGraph
from HardcodedCredentialsFlow::PathNode src, HardcodedCredentialsFlow::PathNode sink
where
HardcodedCredentialsFlow::flowPath(src, sink) and
not any(TestScope test).contains(src.getNode().asCfgNode().getNode())
select src.getNode(), src, sink, "This hardcoded value is $@.", sink.getNode(),
"used as credentials"